[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