[NFC]Fix 2 logic dead code

[XinWang10]

Summary

First, in CodeGenPrepare.cpp, line 6891, the VectorCond will always be false
because if not function will return at 6888.
Second, in SelectionDAGBuilder.cpp, line 5443, getSExtValue() will return
value as int type, but now we use unsigned Val to maintain it, which make the
if condition at 5452 meaningless.

[XinWang10]

rebase

[skan]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5463

Shouldn't we remove the cast too?

[XinWang10]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5463

Sure, it should be.

[XinWang10]

• remove useless cast

[skan]

LGTM

[vzakhari]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5484

The loop hangs on Val equal to -2147483648, because Val never becomes 0.

[vdonaldson]

As noted, it looks like this change is causing compilation of an edge case to hang.

Fortran source:

x = 1.0
print*, x ** (-huge(1)-1)
end

.ll file:

; ModuleID = 'FIRModule'
source_filename = "FIRModule"
target triple = "x86_64-unknown-linux-gnu"

@_QQcl.2F686F6D652F76646F6E616C64736F6E2F742E66393000 = linkonce constant [23 x i8] c"/home/vdonaldson/t.f90\00"
@_QQEnvironmentDefaults = constant ptr null

declare ptr @malloc(i64)

declare void @free(ptr)

define void @_QQmain() !dbg !3 {
  %1 = alloca float, i64 1, align 4, !dbg !7
  store float 1.000000e+00, ptr %1, align 4, !dbg !7, !tbaa !8
  %2 = call ptr @_FortranAioBeginExternalListOutput(i32 -1, ptr @_QQcl.2F686F6D652F76646F6E616C64736F6E2F742E66393000, i32 2), !dbg !12
  %3 = load float, ptr %1, align 4, !dbg !13, !tbaa !8
  %4 = call contract float @llvm.powi.f32.i32(float %3, i32 -2147483648), !dbg !13
  %5 = call i1 @_FortranAioOutputReal32(ptr %2, float %4), !dbg !13
  %6 = call i32 @_FortranAioEndIoStatement(ptr %2), !dbg !12
  ret void, !dbg !14
}

declare ptr @_FortranAioBeginExternalListOutput(i32, ptr, i32)

declare zeroext i1 @_FortranAioOutputReal32(ptr, float)

declare i32 @_FortranAioEndIoStatement(ptr)

; Function Attrs: nocallback nofree nosync nounwind willreturn
declare ptr @llvm.stacksave() #0

; Function Attrs: nocallback nofree nosync nounwind willreturn
declare void @llvm.stackrestore(ptr) #0

; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
declare float @llvm.powi.f32.i32(float, i32) #1

attributes #0 = { nocallback nofree nosync nounwind willreturn }
attributes #1 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }

!llvm.module.flags = !{!0}
!llvm.dbg.cu = !{!1}

!0 = !{i32 2, !"Debug Info Version", i32 3}
!1 = distinct !DICompileUnit(language: DW_LANG_Fortran95, file: !2, producer: "Flang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
!2 = !DIFile(filename: "t.f90", directory: "/home/vdonaldson")
!3 = distinct !DISubprogram(name: "_QQmain", linkageName: "_QQmain", scope: !2, file: !2, line: 1, type: !4, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !1)
!4 = !DISubroutineType(cc: DW_CC_normal, types: !5)
!5 = !{!6, !6}
!6 = !DIBasicType(name: "void", encoding: DW_ATE_address)
!7 = !DILocation(line: 1, column: 1, scope: !3)
!8 = !{!9, !9, i64 0}
!9 = !{!"any data access", !10, i64 0}
!10 = !{!"any access", !11, i64 0}
!11 = !{!"Flang Type TBAA Root"}
!12 = !DILocation(line: 2, column: 1, scope: !3)
!13 = !DILocation(line: 2, column: 9, scope: !3)
!14 = !DILocation(line: 3, column: 1, scope: !3)

[skan]

Thank you for the report @vzakhari @vdonaldson! I fixed it by rGb73229e55543b4ba2b293adcb8b7d6025f01f7d9

[dyung]

In D149033#4311552, @skan wrote:

Thank you for the report @vzakhari @vdonaldson! I fixed it by rGb73229e55543b4ba2b293adcb8b7d6025f01f7d9

This change seems to be causing two test failures on a bot, can you take a look?

https://lab.llvm.org/buildbot/#/builders/247/builds/4041

[skan]

In D149033#4311579, @dyung wrote:

In D149033#4311552, @skan wrote:

Thank you for the report @vzakhari @vdonaldson! I fixed it by rGb73229e55543b4ba2b293adcb8b7d6025f01f7d9

This change seems to be causing two test failures on a bot, can you take a look?

https://lab.llvm.org/buildbot/#/builders/247/builds/4041

Sure. The previous patch was reverted. And I committed a new fix here rG8f966cedea594d9a91e585e88a80a42c04049e6c

[craig.topper]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5463

What was wrong with the original code? The if wasn't meaningless, the cast made it a signed comparison. The unsigned type prevented the undefined behavior on Val = -Val.

[skan]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5463

The original code has one issue. The return value of RHSC->getSExtValue() is int64_t and the minimum can be INT32_MIN, so the implicit cast at line 5454 may be a UB b/c unsigned can not represent the value. So, I fixed it in rG8f966cedea594d9a91e585e88a80a42c04049e6c rather than reverted part of this patch.

[skan]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5463

Sorry, the original code has no issue.

[skan]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5463

What was wrong with the original code? The if wasn't meaningless, the cast made it a signed comparison. The unsigned type prevented the undefined behavior on Val = -Val.

@craig.topper Does the original code have issue at line 5461? The function isBeneficialToExpandPowI takes a int as the first parameter, but if we use unsigned to represent Val, the implicit casting from unsigned to int might be an issue?

llvm/lib/CodeGen/CodeGenPrepare.cpp

bool CodeGenPrepare::optimizeSelectInst(SelectInst *SI) {
 
   bool VectorCond = !SI->getCondition()->getType()->isIntegerTy(1);
 
   // Can we convert the 'select' to CF ?
   if (VectorCond || SI->getMetadata(LLVMContext::MD_unpredictable))
     return false;
 
   TargetLowering::SelectSupportKind SelectKind;
-  if (VectorCond)
-    SelectKind = TargetLowering::VectorMaskSelect;
-  else if (SI->getType()->isVectorTy())
+  if (SI->getType()->isVectorTy())
     SelectKind = TargetLowering::ScalarCondVectorVal;
   else
     SelectKind = TargetLowering::ScalarValSelect;
 
   if (TLI->isSelectSupported(SelectKind) &&
       (!isFormingBranchFromSelectProfitable(TTI, TLI, SI) || OptSize ||
        llvm::shouldOptimizeForSize(SI->getParent(), PSI, BFI.get())))
     return false;

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

 
 /// ExpandPowI - Expand a llvm.powi intrinsic.
 static SDValue ExpandPowI(const SDLoc &DL, SDValue LHS, SDValue RHS,
                           SelectionDAG &DAG) {
   // If RHS is a constant, we can expand this out to a multiplication tree if
   // it's beneficial on the target, otherwise we end up lowering to a call to
   // __powidf2 (for example).
   if (ConstantSDNode *RHSC = dyn_cast<ConstantSDNode>(RHS)) {
-    unsigned Val = RHSC->getSExtValue();
+    int Val = RHSC->getSExtValue();
 
     // powi(x, 0) -> 1.0
     if (Val == 0)
       return DAG.getConstantFP(1.0, DL, LHS.getValueType());
 
     if (DAG.getTargetLoweringInfo().isBeneficialToExpandPowI(
             Val, DAG.shouldOptForSize())) {
       // Get the exponent as a positive value.
-      if ((int)Val < 0)
+      if (Val < 0)
         Val = -Val;
       // We use the simple binary decomposition method to generate the multiply
       // sequence.  There are more optimal ways to do this (for example,
       // powi(x,15) generates one more multiply than it should), but this has
       // the benefit of being both really simple and much better than a libcall.
       SDValue Res; // Logically starts equal to 1.0
       SDValue CurSquare = LHS;
       // TODO: Intrinsics should have fast-math-flags that propagate to these
       // nodes.
       while (Val) {
         if (Val & 1) {
           if (Res.getNode())
             Res =
                 DAG.getNode(ISD::FMUL, DL, Res.getValueType(), Res, CurSquare);
           else
             Res = CurSquare; // 1.0*CurSquare.
         }
 
         CurSquare = DAG.getNode(ISD::FMUL, DL, CurSquare.getValueType(),
                                 CurSquare, CurSquare);
         Val >>= 1;
       }
 
       // If the original was negative, invert the result, producing 1/(x*x*x).
       if (RHSC->getSExtValue() < 0)
         Res = DAG.getNode(ISD::FDIV, DL, LHS.getValueType(),
                           DAG.getConstantFP(1.0, DL, LHS.getValueType()), Res);
       return Res;
     }