[PATCH] D48434: Fix crash on inline asm with 64bit matching input in 32bit GPR

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7222
+  if (OpInfo.isMatchingInputConstraint())
+    return;
+
----------------
efriedma wrote:
> I don't follow this... don't we need to allocate virtual registers anyway?  And how is this relevant to this patch?
The virtual register is allocated later at line 7599 with the patch applied when "looping over all of the inputs, copying the operand values into the appropriate registers and processing the output regs" (dixit comment). That code was already there before any of my patch and I decided to keep that code structure.

It is relevant to this patch because prior to it GetRegistersForValue was not called for matching input since it was only called for C_RegisterClass constraints. It is now called for matching input as well since that is where is the logic to deal with mismatch between register constraint size and the operand type.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7476
+            : ConstraintOperands[i];
     if (OpInfo.ConstraintType == TargetLowering::C_Register)
+      GetRegistersForValue(DAG, TLI, getCurSDLoc(), OpInfo, RefOpInfo);
----------------
efriedma wrote:
> Why is this "OpInfo.ConstraintType == TargetLowering::C_Register" here, but "RefOpInfo.ConstraintType == TargetLowering::C_RegisterClass" later?
I didn't write these two checks but from the comment above the goal seems to distinguish between constraints for specific register (C_Register) from general register constraint (C_RegisterClass). Allocating registers for the former before the latter avoids allocating register X to a general register constraint and later dealing with a constraint asking for X specifically when there was a register Y which the general constraint would have been happy with.


Repository:
  rL LLVM

https://reviews.llvm.org/D48434