[llvm] r263521 - [MachO] Add MachO alt-entry directive support.

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 15 10:40:40 PDT 2016


I don't think this was ready for commit.

Why was it decided that the .alt_entry syntax was needed?

> +
> +    if (MAI->hasAltEntry() && isa<MCBinaryExpr>(Expr))
> +      OutStreamer->EmitSymbolAttribute(Name, MCSA_AltEntry);
> +

I think this is a odd. You are handling "foo" and "foo + 0"
differently. Given what you handle "a = ..." in MC, why do you need to
change codegen at all?

>    DirectiveKindMap[".symbol_resolver"] = DK_SYMBOL_RESOLVER;
> +  DirectiveKindMap[".alt_entry"] = DK_ALT_ENTRY;
>    DirectiveKindMap[".private_extern"] = DK_PRIVATE_EXTERN;
>    DirectiveKindMap[".reference"] = DK_REFERENCE;
>    DirectiveKindMap[".weak_definition"] = DK_WEAK_DEFINITION;

Now you get a crash when using .alt_entry on non MachO. It should be
an error. That is, you should not be parsing this in generic code,
only on MachO specific code.

Probably most importantly:

As noted in the review this introduces a different bug with regards to
ld and llvm-mc not agreeing about which symbols are atoms. Given

bar:
        .alt_entry foo
foo:
call bar

llvm-mc produces a relocation. Given

bar:
foo = .
call bar

we get no relocation. In both cases the linker thinks foo is not an atom.

Cheers,
Rafael


More information about the llvm-commits mailing list