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

Bill Wendling via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jul 27 13:05:51 PDT 2019


void added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7966
     if (OpInfo.ConstraintType == TargetLowering::C_Memory ||
+        OpInfo.ConstraintType == TargetLowering::C_Immediate ||
         OpInfo.ConstraintType == TargetLowering::C_Other) {
----------------
joerg wrote:
> 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?
I wanted to retain the original behavior, but yeah it seems like `C_Immediate` isn't really worth checking here.

As for the `Flags`, wouldn't clearing the flags remove previous iterations which set them? This is being called in a loop over all asm contraints...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60942





More information about the llvm-commits mailing list