[PATCH] D60942: Emit diagnostic if an inline asm constraint requires an immediate

[Joerg Sonnenberger via Phabricator via llvm-commits]

joerg added a comment.

With the exception of the small improvements outlined, this looks good to me.



================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7966
     if (OpInfo.ConstraintType == TargetLowering::C_Memory ||
+        OpInfo.ConstraintType == TargetLowering::C_Immediate ||
         OpInfo.ConstraintType == TargetLowering::C_Other) {
----------------
Unrelated, but shouldn't Flags be filtered first? E.g.

```
Flags &= ~(InlineAsm::Extra_MayLoad | InlineAsm::Extra_MayStore);
```
There seems to be one test case (PowerPC/xray-ret-is-terminate.ll) that depends on this somewhat.

Adding `C_Immediate` here seems to be wrong though, since immediates are never loads or stores. As such, shall we just drop this chunk and bring up the Flags update separate?


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8056
+        OpInfo.CallOperand && !isa<ConstantSDNode>(OpInfo.CallOperand))
+      // We've delayed emitting a diagnostic for the "n" constraint because
+      // inlining could cause an integer showing up.
----------------
`like the "n" constraint`


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:44780
+      return C_Immediate;
+    case 'L':
+    case 'M':
----------------
'L' means 0xFF or 0xFFFF, that should be C_Immediate.



================
Comment at: lib/Target/X86/X86ISelLowering.cpp:44781
+    case 'L':
+    case 'M':
     case 'C':
----------------
'M' means 0..3 as shifts for LEA, should be C_Immediate as well.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60942