[CGExprScalar] In EmitCompare trunc the result if it has different type as E->getType()

[Carrot]

Summary

Usually compare expression should return i1 type, so EmitScalarConversion is called before return

return EmitScalarConversion(Result, CGF.getContext().BoolTy, E->getType(), E->getExprLoc());

But when ppc intrinsic is called to compare vectors, the ppc intrinsic can return i32 even E->getType() is BoolTy, in this case EmitScalarConversion does nothing, an i32 type result is returned and causes crash later.

This patch detects this case and truncates the result before return.

[nemanjai]

I assume this also fixes https://bugs.llvm.org/show_bug.cgi?id=31161?

[Carrot]

I worked on a similar bug as 31161, and then found this one, it should be same as in comment7.
What is the current status of the work on that bug?

[majnemer]

lib/CodeGen/CGExprScalar.cpp

3223–3224 ↗

(On Diff #118114)

Is this clang-format'd ?

3224 ↗

(On Diff #118114)

You are unconditionally dereferencing the result of a dyn_cast. You are either missing a null-check or this should be a cast<>

3225–3226 ↗

(On Diff #118114)

Extra parens.

[nemanjai]

In D38656#892072, @Carrot wrote:

I worked on a similar bug as 31161, and then found this one, it should be same as in comment7.
What is the current status of the work on that bug?

No one has had time to finalize a fix to it. Please go ahead with this patch. If this patch indeed fixes the bug, please close it.

[Carrot]

In D38656#892740, @nemanjai wrote:

In D38656#892072, @Carrot wrote:

I worked on a similar bug as 31161, and then found this one, it should be same as in comment7.
What is the current status of the work on that bug?

No one has had time to finalize a fix to it. Please go ahead with this patch. If this patch indeed fixes the bug, please close it.

This patch fixes the second part of bug 31161 described in comment7. I will check it in soon.
For the first part of the bug, I will retest your patch in comment1, plus incorrect handling of type long in our case, and then send it out.

cfe/trunk/lib/CodeGen/CGExprScalar.cpp

if (LHSTy->isVectorType() && !E->getType()->isVectorType()) {
           std::swap(FirstVecArg, SecondVecArg);
         }
         break;
       }
 
       Value *CR6Param = Builder.getInt32(CR6);
       llvm::Function *F = CGF.CGM.getIntrinsic(ID);
       Result = Builder.CreateCall(F, {CR6Param, FirstVecArg, SecondVecArg});
+
+      // The result type of intrinsic may not be same as E->getType().
+      // If E->getType() is not BoolTy, EmitScalarConversion will do the
+      // conversion work. If E->getType() is BoolTy, EmitScalarConversion will
+      // do nothing, if ResultTy is not i1 at the same time, it will cause
+      // crash later.
+      llvm::IntegerType *ResultTy = cast<llvm::IntegerType>(Result->getType());
+      if (ResultTy->getBitWidth() > 1 &&
+          E->getType() == CGF.getContext().BoolTy)
+        Result = Builder.CreateTrunc(Result, Builder.getInt1Ty());
       return EmitScalarConversion(Result, CGF.getContext().BoolTy, E->getType(),
                                   E->getExprLoc());
     }
 
     if (LHS->getType()->isFPOrFPVectorTy()) {
       Result = Builder.CreateFCmp(FCmpOpc, LHS, RHS, "cmp");
     } else if (LHSTy->hasSignedIntegerRepresentation()) {
       Result = Builder.CreateICmp(SICmpOpc, LHS, RHS, "cmp");

cfe/trunk/test/CodeGen/ppc-vector-compare.cc

+// RUN: %clang_cc1 -target-feature +altivec -triple powerpc64-unknown-unknown -emit-llvm %s \
+// RUN:            -o - | FileCheck %s
+
+#include <altivec.h>
+
+// CHECK-LABEL: @_Z5test1Dv8_tS_
+// CHECK: @llvm.ppc.altivec.vcmpequh.p
+bool test1(vector unsigned short v1, vector unsigned short v2) {
+  return v1 == v2;
+}
+