3

I've got a project in Xcode (4.5.2) that builds fine using the Debug configuration. However, now that I've switched to building the Release configuration, I'm getting an issue: one of my inline assembly functions is getting the error Invalid symbol redefinition. Googling that error message finds me a few people who have got that compiler error, but no information as to what it means. Here's the function, with the error lines annotated:

inline int MulDivAdd(int nNumber,
                int nNumerator,
                int nDenominator,
                int nToAdd)
{
    int nRet;

    __asm__ __volatile__ (
        "mov    %4,     %%ecx   \n"
        "mov    %1,     %%eax   \n"
        "mull   %2              \n"
        "cmp    $0,     %%ecx   \n"
        "jl     __sub           \n"
        "addl   %%ecx,  %%eax   \n"
        "adc    $0,     %%edx   \n"
        "jmp    __div           \n"
    "__sub:                     \n"    // "Invalid symbol redefinition"
        "neg    %%ecx           \n"
        "subl   %%ecx,  %%eax   \n"
        "sbb    $0,     %%edx   \n"
    "__div:                     \n"    // "Invalid symbol redefinition"
        "divl   %3              \n"
        "mov    %%eax,  %0      \n"

        :   "=m"    (nRet)
        :   "m"     (nNumber),
            "m"     (nNumerator),
            "m"     (nDenominator),
            "m"     (nToAdd)
        :   "eax", "ecx", "edx"

    );

    return nRet;
}

I've tried replacing __sub with __sbt because I thought __sub might be a protected name, but that wasn't the case. I don't understand why this only happens in Release though - could it be due to optimisation?

benwad
  • 6,414
  • 10
  • 59
  • 93
  • 1
    If you write this in C (using `int64_t`) you'll get better codegen. The branch is entirely unnecessary (it should be a sign-extend and add). I'm also troubled by the use of an unsigned multiply instruction (`mull`) with signed data. While it may not matter for the program in question, it's probably a bug. – Stephen Canon Jan 24 '13 at 16:47
  • @StephenCanon So how is this wisecracking related to the question? – Michael Lehn May 23 '14 at 18:53
  • This doesn't need to be `volatile`. It only needs to run to produce the outputs. (And `volatile` doesn't stop it from inlining into multiple places, causing the problem). But Stephen is right, this looks terrible for efficiency with forced memory operands. The only reason to use inline asm here is if you know the 64b/32b => 32b `div` won't overflow the quotient and fault. gccl/clang still don't know how to take advantage of that and use a single `idiv` instead of calling a libgcc helper function for 64b/64b division. – Peter Cordes Apr 24 '18 at 09:33

1 Answers1

13

Use local labels, such as 1: and 2:, and jxx 1f or jxx 1b. The direction of the jump (f for forwards or b for backwards) is required. So your code should be like this:

__asm__ __volatile__ (
    "mov    %4,     %%ecx   \n"
    "mov    %1,     %%eax   \n"
    "mull   %2              \n"
    "cmp    $0,     %%ecx   \n"
    "jl     1f              \n"
    "addl   %%ecx,  %%eax   \n"
    "adc    $0,     %%edx   \n"
    "jmp    2f              \n"
"1:                         \n"   
    "neg    %%ecx           \n"
    "subl   %%ecx,  %%eax   \n"
    "sbb    $0,     %%edx   \n"
"2:                         \n"   
    "divl   %3              \n"
    "mov    %%eax,  %0      \n"
)

Symbols consisting purely of numbers are "local to the function". Since "inline" means the code is physically duplicated, the reason you get multiple symbol definitions is that your symbols are indeed defined multiple times in a "global" way.

Of course, if you have a debug build, it normally means "no inline", so the inlined function isn't inlined, symbols only declared once, and it "works".

[I'm slightly dubious as to the efficiency of this vs. what the compiler would do itself - I'd have thought at least considering using registers for some of the inputs would make it more efficient].

jww
  • 97,681
  • 90
  • 411
  • 885
Mats Petersson
  • 126,704
  • 14
  • 140
  • 227
  • Thanks for the quick answer! As for the efficiency, this is something I've been porting so I was just translating the original Intel syntax ASM into GAS. I may try to optimise it in the future though. – benwad Jan 24 '13 at 16:43
  • 1
    I would be tempted to rewrite it as C first, and see how that goes. [I'm not 100% sure what it does, but it looks like part of a large number math function - but that could be completely wrong] – Mats Petersson Jan 24 '13 at 16:45
  • You might explain what the `f` and `b` suffixes on the jump targets mean =) – Stephen Canon Jan 24 '13 at 16:48
  • @Mats- *"Use numeric labels..."* - I think those are called ***local labels***. See [Labels in GCC inline assembly](https://stackoverflow.com/questions/3898435/labels-in-gcc-inline-assembly/3898451#3898451). – jww Jul 21 '15 at 16:36
  • They're not actually local to the function. If you rearrange your code without correcting the `f`orward and `b`ack directions in the label references, you could jump to the `1:` in the previous copy of the inline-asm block. And gas doesn't support `1$:` limited-scope local label names for x86 targets. For reference: [the gas manual](https://sourceware.org/binutils/docs/as/Symbol-Names.html). – Peter Cordes Sep 20 '16 at 21:53