[llvm] r337903 - Fix PR34170: Crash on inline asm with 64bit output in 32bit GPR

Artem Belevich via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 2 15:54:49 PDT 2018


Hi,

It appears that this patch crashes llvm under some circumstances. The crash
goes away if the patch is reverted.

Reproduction:

test.cc:

```
float asmfunc(int a, int b) {
  float read_value;
  asm("" : "=r"(read_value), "=r"(a));
  return read_value;
}
int B_X;
void kFCNorm(int a, float b) {
  B_X = asmfunc(a, B_X) * b;
}
```

```
$ clang -S -O3 test.cc -o - `
...
clang-7:
<redacted>/repo/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:4489:
llvm::SDValue llvm::SelectionDAG::getNode(unsigned int, const llvm::SDLoc
&, llvm::EVT, llvm::SDValue, llvm::SDValue, const llvm::SDNodeFlags):
Assertion `VT.isFloatingPoint() && "This operator only applies to FP
types!"' failed.
```

On Wed, Jul 25, 2018 at 4:11 AM Thomas Preud'homme via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: thopre
> Date: Wed Jul 25 04:11:12 2018
> New Revision: 337903
>
> URL: http://llvm.org/viewvc/llvm-project?rev=337903&view=rev
> Log:
> Fix PR34170: Crash on inline asm with 64bit output in 32bit GPR
>
> Add support for inline assembly with output operand that do not
> naturally go in the register class it is constrained to (eg. double in a
> 32-bit GPR as in the PR).
>
> Added:
>     llvm/trunk/test/CodeGen/ARM/inline-asm-operand-implicit-cast.ll
> Modified:
>     llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=337903&r1=337902&r2=337903&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Wed Jul 25
> 04:11:12 2018
> @@ -7185,27 +7185,37 @@ static void GetRegistersForValue(Selecti
>
>    unsigned NumRegs = 1;
>    if (OpInfo.ConstraintVT != MVT::Other) {
> -    // If this is a FP input in an integer register (or visa versa)
> insert a bit
> -    // cast of the input value.  More generally, handle any case where
> the input
> -    // value disagrees with the register class we plan to stick this in.
> -    if (OpInfo.Type == InlineAsm::isInput && PhysReg.second &&
> +    // If this is a FP operand in an integer register (or visa versa), or
> more
> +    // generally if the operand value disagrees with the register class
> we plan
> +    // to stick it in, fix the operand type.
> +    //
> +    // If this is an input value, the bitcast to the new type is done now.
> +    // Bitcast for output value is done at the end of visitInlineAsm().
> +    if ((OpInfo.Type == InlineAsm::isOutput ||
> +         OpInfo.Type == InlineAsm::isInput) &&
> +        PhysReg.second &&
>          !TRI.isTypeLegalForClass(*PhysReg.second, OpInfo.ConstraintVT)) {
>        // Try to convert to the first EVT that the reg class contains.  If
> the
>        // types are identical size, use a bitcast to convert (e.g. two
> differing
> -      // vector types).
> +      // vector types).  Note: output bitcast is done at the end of
> +      // visitInlineAsm().
>        MVT RegVT = *TRI.legalclasstypes_begin(*PhysReg.second);
> -      if (RegVT.getSizeInBits() ==
> OpInfo.CallOperand.getValueSizeInBits()) {
> -        OpInfo.CallOperand = DAG.getNode(ISD::BITCAST, DL,
> -                                         RegVT, OpInfo.CallOperand);
> +      if (RegVT.getSizeInBits() == OpInfo.ConstraintVT.getSizeInBits()) {
> +        // Exclude indirect inputs while they are unsupported because the
> code
> +        // to perform the load is missing and thus OpInfo.CallOperand
> still
> +        // refer to the input address rather than the pointed-to value.
> +        if (OpInfo.Type == InlineAsm::isInput && !OpInfo.isIndirect)
> +          OpInfo.CallOperand =
> +              DAG.getNode(ISD::BITCAST, DL, RegVT, OpInfo.CallOperand);
>          OpInfo.ConstraintVT = RegVT;
> +        // If the operand is a FP value and we want it in integer
> registers,
> +        // use the corresponding integer type. This turns an f64 value
> into
> +        // i64, which can be passed with two i32 values on a 32-bit
> machine.
>        } else if (RegVT.isInteger() &&
> OpInfo.ConstraintVT.isFloatingPoint()) {
> -        // If the input is a FP value and we want it in FP registers, do a
> -        // bitcast to the corresponding integer type.  This turns an f64
> value
> -        // into i64, which can be passed with two i32 values on a 32-bit
> -        // machine.
>          RegVT = MVT::getIntegerVT(OpInfo.ConstraintVT.getSizeInBits());
> -        OpInfo.CallOperand = DAG.getNode(ISD::BITCAST, DL,
> -                                         RegVT, OpInfo.CallOperand);
> +        if (OpInfo.Type == InlineAsm::isInput)
> +          OpInfo.CallOperand =
> +              DAG.getNode(ISD::BITCAST, DL, RegVT, OpInfo.CallOperand);
>          OpInfo.ConstraintVT = RegVT;
>        }
>      }
> @@ -7717,12 +7727,18 @@ void SelectionDAGBuilder::visitInlineAsm
>      if (CS.getType()->isSingleValueType() && CS.getType()->isSized()) {
>        EVT ResultType = TLI.getValueType(DAG.getDataLayout(),
> CS.getType());
>
> -      // If any of the results of the inline asm is a vector, it may have
> the
> -      // wrong width/num elts.  This can happen for register classes that
> can
> -      // contain multiple different value types.  The preg or vreg
> allocated may
> -      // not have the same VT as was expected.  Convert it to the right
> type
> -      // with bit_convert.
> -      if (ResultType != Val.getValueType() &&
> Val.getValueType().isVector()) {
> +      // If the type of the inline asm call site return value is
> different but
> +      // has same size as the type of the asm output bitcast it.  One
> example
> +      // of this is for vectors with different width / number of elements.
> +      // This can happen for register classes that can contain multiple
> +      // different value types.  The preg or vreg allocated may not have
> the
> +      // same VT as was expected.
> +      //
> +      // This can also happen for a return value that disagrees with the
> +      // register class it is put in, eg. a double in a general-purpose
> +      // register on a 32-bit machine.
> +      if (ResultType != Val.getValueType() &&
> +          ResultType.getSizeInBits() == Val.getValueSizeInBits()) {
>          Val = DAG.getNode(ISD::BITCAST, getCurSDLoc(),
>                            ResultType, Val);
>
>
> Added: llvm/trunk/test/CodeGen/ARM/inline-asm-operand-implicit-cast.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/inline-asm-operand-implicit-cast.ll?rev=337903&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/inline-asm-operand-implicit-cast.ll (added)
> +++ llvm/trunk/test/CodeGen/ARM/inline-asm-operand-implicit-cast.ll Wed
> Jul 25 04:11:12 2018
> @@ -0,0 +1,42 @@
> +; RUN: llc -mtriple armv7-arm-linux-gnueabihf -O2 -mcpu=cortex-a7 < %s |
> FileCheck %s
> +
> +; Check support for returning a float in GPR with soft float ABI
> +define arm_aapcscc float @zerobits_float_soft() #0 {
> +; CHECK-LABEL: zerobits_float_soft
> +; CHECK: mov r0, #0
> +  %1 = tail call float asm "mov ${0}, #0", "=&r"()
> +  ret float %1
> +}
> +
> +; Check support for returning a double in GPR with soft float ABI
> +define arm_aapcscc double @zerobits_double_soft() #0 {
> +; CHECK-LABEL: zerobits_double_soft
> +; CHECK: mov r0, #0
> +; CHECK-NEXT: mov r1, #0
> +  %1 = tail call double asm "mov ${0:Q}, #0\0Amov ${0:R}, #0", "=&r"()
> +  ret double %1
> +}
> +
> +attributes #0 = { nounwind
> "target-features"="+d16,+vfp2,+vfp3,-fp-only-sp" "use-soft-float"="true" }
> +
> +
> +; Check support for returning a float in GPR with hard float ABI
> +define float @zerobits_float_hard() #1 {
> +; CHECK-LABEL: zerobits_float_hard
> +; CHECK: mov r0, #0
> +; CHECK: vmov s0, r0
> +  %1 = tail call float asm "mov ${0}, #0", "=&r"()
> +  ret float %1
> +}
> +
> +; Check support for returning a double in GPR with hard float ABI
> +define double @zerobits_double_hard() #1 {
> +; CHECK-LABEL: zerobits_double_hard
> +; CHECK: mov r0, #0
> +; CHECK-NEXT: mov r1, #0
> +; CHECK: vmov d0, r0, r1
> +  %1 = tail call double asm "mov ${0:Q}, #0\0Amov ${0:R}, #0", "=&r"()
> +  ret double %1
> +}
> +
> +attributes #1 = { nounwind
> "target-features"="+d16,+vfp2,+vfp3,-fp-only-sp" "use-soft-float"="false" }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>


-- 
--Artem Belevich
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180802/5f855e6d/attachment.html>


More information about the llvm-commits mailing list