[llvm-ml] Add support for extern proc

[ayzhao]

Summary

EXTERN PROC isn't really well documented in MSVC, so after poking around
it seems as if it's just a regular extern symbol.

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.

[epastor]

llvm/lib/MC/MCParser/MasmParser.cpp

6038

Can we check this before doing the lookup? It seems odd to do the complicated check first.

[ayzhao]

Reorder if statements

[epastor]

llvm/test/tools/llvm-ml/indirect_branch.asm

270

This (and the label) could probably be hoisted outside of the ifdef.

[ayzhao]

move label outside ifdef

[hans]

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.

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.

[ayzhao]

In 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.

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.

[ayzhao]

In D125582#3516950, @ayzhao wrote:

In 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.

[hans]

Okay, but even if llvm-ml's output is better, it should still have a test. And since it differs from MSVC's output an error or at least a warning is probably a good idea.

llvm/lib/MC/MCParser/MasmParser.cpp

if (parseIdentifier(Name))
       return Error(NameLoc, "expected name");
     if (parseToken(AsmToken::Colon))
       return true;
 
     StringRef TypeName;
     SMLoc TypeLoc = getTok().getLoc();
     if (parseIdentifier(TypeName))
       return Error(TypeLoc, "expected type");
+    if (!TypeName.equals_insensitive("proc")) {
       AsmTypeInfo Type;
       if (lookUpType(TypeName, Type))
         return Error(TypeLoc, "unrecognized type");
       KnownType[Name.lower()] = Type;
+    }
 
     MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
     Sym->setExternal(true);
     getStreamer().emitSymbolAttribute(Sym, MCSA_Extern);
 
     return false;
   };
 

llvm/test/tools/llvm-ml/extern.asm

 ; RUN: llvm-ml -m32 -filetype=s %s /Fo - | FileCheck %s --check-prefixes=CHECK,CHECK-32
 ; RUN: llvm-ml -m64 -filetype=s %s /Fo - | FileCheck %s --check-prefixes=CHECK,CHECK-64
 
-extern foo : dword, bar : word
+extern foo : dword, bar : word, baz : proc
 ; CHECK: .extern foo
 ; CHECK: .extern bar
+; CHECK: .extern baz
 
 .code
 mov ebx, foo
 ; CHECK-32: mov ebx, dword ptr [foo]
 ; CHECK-64: mov ebx, dword ptr [rip + foo]
 
 mov bx, bar
 ; CHECK-32: mov bx, word ptr [bar]
 ; CHECK-64: mov bx, word ptr [rip + bar]
 
 END

llvm/test/tools/llvm-ml/indirect_branch.asm

 ; RUN: llvm-ml -m64 -filetype=s %s /Fo - | FileCheck %s --check-prefixes=CHECK-64,CHECK
 ; RUN: llvm-ml -m32 -filetype=s %s /Fo - | FileCheck %s --check-prefixes=CHECK-32,CHECK
 
 ifdef rax
   extern fn_ref_extern : qword
 else
   extern fn_ref_extern : dword
 endif
+
+extern fn_proc_extern : proc
 extern fn_ref_extern_word : word
 
 .data
 
 ifdef rax
   fn_ref qword 1
 else
   fn_ref dword 1
else
 
   t22:
   call word ptr [fn_ref_extern_word]
   jmp word ptr [fn_ref_extern_word]
   ; CHECK-32-LABEL: t22:
   ; CHECK-32: call word ptr [fn_ref_extern_word]
   ; CHECK-32-NEXT: jmp word ptr [fn_ref_extern_word]
 endif
+
+t23:
+call fn_proc_extern
+jmp fn_proc_extern
+; CHECK-LABEL: t23:
+; CHECK: call fn_proc_extern
+
+t24:
+call [fn_proc_extern]
+jmp [fn_proc_extern]
+; CHECK-LABEL: t24:
+; CHECK: call fn_proc_extern
+
+t25:
+; CHECK-LABEL: t25:
+ifdef rax
+  call qword ptr [fn_ref_extern]
+  jmp qword ptr [fn_ref_extern]
+  ; CHECK-64: call qword ptr [rip + fn_ref_extern]
+  ; CHECK-64: jmp qword ptr [rip + fn_ref_extern]
+else
+  call dword ptr [fn_ref_extern]
+  jmp dword ptr [fn_ref_extern]
+  ; CHECK-32: call dword ptr [fn_ref_extern]
+  ; CHECK-32: jmp dword ptr [fn_ref_extern]
+endif
+
+end