<div dir="ltr">Hi Rafael,<div><br></div><div>Apologies - mis-hit the send button. Continuing on... </div><div><br></div><div><span style="font-size:13px">My patch special-cased 'foo + 0' to behave the same as 'foo' - ld64 recognizes aliases for the same location and treats them as one atom. We only need to set the alt_entry bit on 'foo + C', where C is non-zero.</span></div><div><span style="font-size:13px"><br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span style="font-size:13px">As noted in the review this introduces a different bug with regards to<br></span><span style="font-size:13px">ld and llvm-mc not agreeing about which symbols are atoms.</span></blockquote><div><br></div><div>This is unfortunate, but the new bug is benign: at worst you get redundant relocations. With a bit of work in MC I can probably get rid of these too. The old bug was much more dangerous - you could end up with code that needed relocations and didn't have any.</div><div><br></div><div>- Lang. </div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 22, 2016 at 8:14 AM, Lang Hames <span dir="ltr"><<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Hi Rafael,<br></div><span class=""><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span style="font-size:13px">Why was it decided that the .alt_entry syntax was needed?</span></blockquote><div class="gmail_extra"><br></div></span><div class="gmail_extra">The majority of the MachO assembly users felt it was clearer syntax. This should be MachO specific though - I'll take a look and either change where this is parsed, or issue an error if it's parsed for a non-MachO file.</div><span class=""><div class="gmail_extra"><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span style="font-size:13px">I think this is a odd. You are handling "foo" and "foo + 0" </span><span style="font-size:13px">differently. </span></blockquote><div class="gmail_extra"><br></div></span><div class="gmail_extra">I believe 'foo + 0' is special-cased to behave the same as 'foo</div><div><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 15, 2016 at 10:40 AM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">I don't think this was ready for commit.<br>
<br>
Why was it decided that the .alt_entry syntax was needed?<br>
<span><br>
> +<br>
> +    if (MAI->hasAltEntry() && isa<MCBinaryExpr>(Expr))<br>
> +      OutStreamer->EmitSymbolAttribute(Name, MCSA_AltEntry);<br>
> +<br>
<br>
</span>I think this is a odd. You are handling "foo" and "foo + 0"<br>
differently. Given what you handle "a = ..." in MC, why do you need to<br>
change codegen at all?<br>
<span><br>
>    DirectiveKindMap[".symbol_resolver"] = DK_SYMBOL_RESOLVER;<br>
> +  DirectiveKindMap[".alt_entry"] = DK_ALT_ENTRY;<br>
>    DirectiveKindMap[".private_extern"] = DK_PRIVATE_EXTERN;<br>
>    DirectiveKindMap[".reference"] = DK_REFERENCE;<br>
>    DirectiveKindMap[".weak_definition"] = DK_WEAK_DEFINITION;<br>
<br>
</span>Now you get a crash when using .alt_entry on non MachO. It should be<br>
an error. That is, you should not be parsing this in generic code,<br>
only on MachO specific code.<br>
<br>
Probably most importantly:<br>
<br>
As noted in the review this introduces a different bug with regards to<br>
ld and llvm-mc not agreeing about which symbols are atoms. Given<br>
<br>
bar:<br>
        .alt_entry foo<br>
foo:<br>
call bar<br>
<br>
llvm-mc produces a relocation. Given<br>
<br>
bar:<br>
foo = .<br>
call bar<br>
<br>
we get no relocation. In both cases the linker thinks foo is not an atom.<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div></div></div></div>
</blockquote></div><br></div>