[PATCH] D125582: [llvm-ml] Add support for extern proc

[Alan Zhao via Phabricator via llvm-commits]

ayzhao added a comment.

In D125582#3516950 <https://reviews.llvm.org/D125582#3516950>, @ayzhao wrote:

> In D125582#3515465 <https://reviews.llvm.org/D125582#3515465>, @hans wrote:
>
>>> Interestingly enough, under MSVC the following is allowed:
>>>
>>> extern foo:proc
>>>
>>> mov eax, foo
>>>
>>> MSVC will output:
>>>
>>> mov eax, 0
>>>
>>> while llvm-ml will currently output:
>>>
>>> mov eax, dword ptr [foo]
>>>
>>> (since foo is an extern)
>>>
>>> Arguably, llvm-ml's output makes more sense, even though it's
>>>  inconsistent with MSVC ml. However, since moving an extern proc symbol
>>>  to a register doesn't really make sense in the first place, we'll treat
>>>  it as undefined behavior for now.
>>
>> Maybe I'm just not familiar enough with MASM, but both outputs seem odd to me. I would expect MSVC's output to be accompanied by a relocation, but it seems that's not the case?
>>
>> "mov eax, foo" reads like "move the address of foo into eax" to me, which seems like it would be a sensible thing to do. What llvm-ml does is more like "load from foo into eax" which seems odd if "foo" is a function.
>
> It seems that the instruction for "move the address of foo into eax" should have a rip relative address anyways, just because `foo` is an extern. See: https://godbolt.org/z/Wsjq3KdvT
>
> Based on that, I think llvm-ml is doing the right thing currently.

Better godbolt link that removes some unwanted inlining: https://godbolt.org/z/Kz4oTeYG5

>> In any case, if we can't support it in a sensible way, can we at least error about it? And there should be a test.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125582/new/

https://reviews.llvm.org/D125582