1

Hi i was wandering if using goto is good practice for optimization. i know it's a barbarian thing to do but, seriously.

for instance write this:

switch(command[0].cmd)
{
    case 0: // turn off
        s::s_off();
        S_time = command[0].length;
    break;
    case 1: // turn on
        s::s_on();
        S_time = command[0].length;
    break;
    case 4: // nop

    break;
}

like this:

switch(command[0].cmd)
{
    case 0: // turn off
        s::s_off();
        goto a;
    break;
    case 1: // turn on
        s::s_on();
        goto a;
    break;
    case 4: // nop
        goto b;
    break;
}

a:
S_time = command[0].length;
b:
Christian Hackl
  • 27,051
  • 3
  • 32
  • 62
  • 3
    No. There are very, very, very few cases where you actuall need to use `goto` and it's not for optimization. Probably related: http://stackoverflow.com/questions/379172/use-goto-or-not – UnholySheep Jan 12 '17 at 09:03
  • *"i know it's a barbarian thing to do"* - Have you actually read Dijkstra's famous paper, or are you going by the title alone as most people do? – StoryTeller - Unslander Monica Jan 12 '17 at 09:03
  • 2
    No. It's almost never a good idea, even for "optimization". – Some programmer dude Jan 12 '17 at 09:03
  • 2
    Ever heard of functions? – Kamil Koczurek Jan 12 '17 at 09:03
  • In this particular case it's probably better to extract bodies of `case 0` and `case 1` as a function – alexeykuzmin0 Jan 12 '17 at 09:04
  • @StoryTeller no but i'll give it a try :D –  Jan 12 '17 at 09:04
  • In the specific case with the code shown, some are recommending breaking out the common code into a function. I instead think you should refactor the code as `if` statements, and then just let the compiler handle the optimizations needed (which it might handle just fine even with the `switch` statement, look at the generated code with compiler optimizations enabled). – Some programmer dude Jan 12 '17 at 09:06
  • 3
    Good when you do. Keep goto in your tool box for the very rare occasion you'd really need it (this isn't it, and in C++ you'll unlikely to ever). If this was C, there are some cases where goto's actually make code better (by being used in a structured manner). – StoryTeller - Unslander Monica Jan 12 '17 at 09:06
  • 1
    @StoryTeller for illustration purposes I think this article sums it up quite nicely: http://eli.thegreenplace.net/2009/04/27/using-goto-for-error-handling-in-c – CodeMonkey Jan 12 '17 at 09:11
  • No its not a good Idea but not evil though.. Any specific reason why you considered `GOTO` – Daksh Gupta Jan 12 '17 at 09:11

4 Answers4

5

Indeed it is wise to avoid goto if possible, and trust the compiler to make the optimisations for you. Even in your case there is an alternative, which avoids the code duplication:

/*possibly inline*/ void foo(/*pass necessary parameters*/)
{
    switch(command[0].cmd){
    case 0: // turn off
        s::s_off();
        break;
    case 1: // turn on
        s::s_on();
        break;
    case 4: // nop
        return;       
    }
    S_time = command[0].length;
}
Bathsheba
  • 231,907
  • 34
  • 361
  • 483
  • 1
    That return statement is a huge assumption that the OP has no further processing to do. Not bad advise otherwise. – StoryTeller - Unslander Monica Jan 12 '17 at 09:11
  • there's like 50 other things that are similar only for few cases, it could really benefit from optimization, i have 90% of MCU full, and that's not a good thing, since i only started. –  Jan 12 '17 at 09:26
2

As far as optimizations go, profiling is the best you can do. But let's have a look at the generated assembly nonetheless, as that can be useful too. I'll use the following dummy declarations:

namespace s {
    void s_on();
    void s_off();
};

struct Command {
    int cmd;
    int length;
};

int S_time;

The first version of your code compiled with Clang at -O3 produces:

foo(Command*):                        # @foo(Command*)
        push    rbx
        mov     rbx, rdi
        mov     eax, dword ptr [rbx]
        cmp     eax, 1
        je      .LBB0_3
        test    eax, eax
        jne     .LBB0_5
        call    s::s_off()
        jmp     .LBB0_4
.LBB0_3:
        call    s::s_on()
.LBB0_4:
        mov     eax, dword ptr [rbx + 4]
        mov     dword ptr [rip + S_time], eax
.LBB0_5:
        pop     rbx
        ret

While the second version, with goto, produces:

foo2(Command*):                       # @foo2(Command*)
        push    rbx
        mov     rbx, rdi
        mov     eax, dword ptr [rbx]
        cmp     eax, 4
        je      .LBB1_6
        cmp     eax, 1                # These two instructions
        je      .LBB1_4               # weren't here in the first version
        test    eax, eax
        jne     .LBB1_5
        call    s::s_off()
        jmp     .LBB1_5
.LBB1_4:
        call    s::s_on()
.LBB1_5:
        mov     eax, dword ptr [rbx + 4]
        mov     dword ptr [rip + S_time], eax
.LBB1_6:
        pop     rbx
        ret

Not as clear as some other cases, but there is one difference: the first version only compares command[0].cmd with 0 and 1, but the second one compares it with 0, 1 and 4. Less code repetition does not necessarily mean more optimised code: you have actually hindered the optimiser and made it generate a useless special case for 4.

goto is not the low-level tool imbued with a magic low-level optimisation aura that its detractors describe. It's just a very basic flow control tool, which has few (but still some!) uses in C++ when the other tools don't cut it. It can, of course, be used for optimisation, but no better or more easily than any of the other ones.

Quentin
  • 62,093
  • 7
  • 131
  • 191
  • thank's but i'm using atmel avr, so the assembler is different, also there is a ton of code that get's used, plus -O3 produces smaller size in avr GCC, but the code runs poorly, so I'm using just general optimalization. :D –  Jan 12 '17 at 09:37
  • also I'm doing checks for performance and size of final assembly, it's cool to see someone who does that too. :D –  Jan 12 '17 at 09:41
0

It is not recommended to use goto frequently. But it is not "evil" and in many cases is much easier to put 1 goto instead of more complex logic only to avoid the "evil". And in your example single goto is enough:

switch(command[0].cmd)
{
    case 0: // turn off
        s::s_off();
        break;

    case 1: // turn on
        s::s_on();
        break;

    case 4: // nop
        goto b; // break after goto is useless
}

S_time = command[0].length;
b:
i486
  • 6,491
  • 4
  • 24
  • 41
  • 2
    This is largely personal but I would fail this in a code review: it's not necessary *enough*. But you do set this out well, plus one. – Bathsheba Jan 12 '17 at 09:10
  • It's `goto`, not `GOTO`. C++ keywords are case-sensitive. – Christian Hackl Jan 12 '17 at 10:59
  • Sorry, but the goto in the code above is not necessary, but evil. I write code for almost 20 years now (13+ years as a profession) and have never seen a code segment where goto is a necessity, but I can imagine one, but this simply isn't the case. – mg30rg Jan 12 '17 at 16:28
  • @mg30rg `goto` above is modification of author's example with minimal changes. I write code for almost 30 years if this is important. And ask yourself - not only "Why `goto` exists in C" but also "Why there is no Warning when using `goto`"? Because it is not evil :) And may help in many cases. – i486 Jan 12 '17 at 20:30
  • @i486 Minimal changes are important mainly for version tracking,and the OP's code would never make it into production code at any sane enterprise, so it doesn't need to be changed in an easily traceable manner. Goto exist in C for legacy reasons - there was a time when the compiler wasn't smart enough, and the coder had to do the micro-optimization. There is no warning for _using goto_,because of legacy code. Most of the places I worked at there was a policy not to compile production code without "treat warnings as errors",so all legacy libraries should be refactored because of such a warning. – mg30rg Jan 16 '17 at 08:51
  • @mg30rg C++ compilers are newer than C. And there again (I think) is no warning for `goto`. The "problem" with `goto` comes from academic community and teachers without practical experience as programmers. – i486 Jan 16 '17 at 09:48
  • @i486 I'm no information scientist, but a humble coder with 13+ years of field experience and I only found use for `goto` in languages where no proper _loop management_ **and/or** (multiple) _selection management_ were available. I repeat, I can **imagine** a use-case for goto, but I never found one in well-organized code. – mg30rg Jan 16 '17 at 10:39
  • @i486 Also, C++ is newer than C only by some five or six years, and its not uncommon to see inline C code in legacy C++ applications. There are also library routines written before smart compilers and never refactored using goto-heavy code for sorting and searching routines, which wouldn't worth rewriting. – mg30rg Jan 17 '17 at 13:43
0

The use of goto should be to make the code clearer. Not more complex. This is why for example goto out of a deeply nested set of loops or if-conditions would be acceptable sometimes, but using goto to "optimise" the code like in the example in this question is not a particularly good cause - it doesn't make the code clearer, rather the other way.

There are OFTEN other solutions - breaking something into smaller functions [often a good thing anyway] or restructuring the code. But if you end up with more complex code thanks to "avoid goto at all costs", then you are probably going the wrong way.

This question exemplifies a case where using goto MAY indeed be a better choice than the one chosen in the question itself: What are some better ways to avoid the do-while(0); hack in C++?

Community
  • 1
  • 1
Mats Petersson
  • 126,704
  • 14
  • 140
  • 227