[PATCH] D33277: [Clang][x86][Inline Asm] - Enum support for MS syntax

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp:12
+  const int a = 0;
+  // CHECK-NOT: mov eax, [$$0]
+  __asm mov eax, [a]
----------------
rnk wrote:
> mharoush wrote:
> > rnk wrote:
> > > Use CHECK-LABEL, CHECK, and CHECK-SAME the way that the existing ms-inline-asm.c tests do so that this test is easier to debug when it fails.
> > This test case was just meant verify that other Integer constants are not folded since we get a different behavior for statements such as mov eax, [a]. 
> > #---
> > In this example X86AsmParser regards the address of the variable 'a' and not its value i.e. we end up with the value of 'a' in eax (loaded from the stack) and not with the value pointed by the const int value of 'a' as its address.
> > ---#
> > 
> > I can clarify the intention in a comment or completely remove the test case since this isn't really required here.
> The test case is fine and the intention is clear, I am just suggesting that you use more FileCheck features (CHECK-LABEL and CHECK-SAME) to make the test easier to debug when it fails in the future.
You didn't implement this suggestion, please take a look at ms-inline-asm.c. It has tests that look like this:
```
void t13() {
  char i = 1;
  short j = 2;
  __asm movzx eax, i
  __asm movzx eax, j
// CHECK-LABEL: define void @t13()
// CHECK: call void asm sideeffect inteldialect
// CHECK-SAME: movzx eax, byte ptr $0
// CHECK-SAME: movzx eax, word ptr $1
// CHECK-SAME: "*m,*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i8* %{{.*}}i, i16* %{{.*}}j)
}
```

This is much less error-prone and easier to debug when it fails.


Repository:
  rL LLVM

https://reviews.llvm.org/D33277