[SimplifyCFG] Use a more generic name for the selects created by SpeculativelyExecuteBB to prevent long names from being created

[craig.topper]

Summary

Currently the selects are created with the names of their inputs concatenated together. It's possible to get cases that chain these selects together resulting in long names due to multiple levels of concatenation. Our internal branch of llvm managed to generate names over 100000 characters in length on a particular test due to an extreme compounding of the names.

This patch changes the name to a generic name that is not dependent on its inputs. I'm open to other suggestions.

As a small demonstration, here is what is currently in the output IR for merge-cond-stores-2.ll before this patch

%1 = select i1 %tobool, i32 %0, i32 %or
%.or4 = select i1 %tobool2, i32 %1, i32 %or4
%.or4.or9 = select i1 %tobool7, i32 %.or4, i32 %or9
%.or4.or9.or14 = select i1 %tobool12, i32 %.or4.or9, i32 %or14
%.or4.or9.or14.or19 = select i1 %tobool17, i32 %.or4.or9.or14, i32 %or19
%.or4.or9.or14.or19.or24 = select i1 %tobool22, i32 %.or4.or9.or14.or19, i32 %or24
%.or4.or9.or14.or19.or24.or29 = select i1 %tobool27, i32 %.or4.or9.or14.or19.or24, i32 %or29
%.or4.or9.or14.or19.or24.or29.or34 = select i1 %tobool32, i32 %.or4.or9.or14.or19.or24.or29, i32 %or34
%.or4.or9.or14.or19.or24.or29.or34.or39 = select i1 %tobool37, i32 %.or4.or9.or14.or19.or24.or29.or34, i32 %or39
%.or4.or9.or14.or19.or24.or29.or34.or39.or44 = select i1 %tobool42, i32 %.or4.or9.or14.or19.or24.or29.or34.or39, i32 %or44
%.or4.or9.or14.or19.or24.or29.or34.or39.or44.or49 = select i1 %tobool47, i32 %.or4.or9.or14.or19.or24.or29.or34.or39.or44, i32 %or49
%.or4.or9.or14.or19.or24.or29.or34.or39.or44.or49.or54 = select i1 %tobool52, i32 %.or4.or9.or14.or19.or24.or29.or34.or39.or44.or49, i32 %or54
%.or4.or9.or14.or19.or24.or29.or34.or39.or44.or49.or54.or59 = select i1 %tobool57, i32 %.or4.or9.or14.or19.or24.or29.or34.or39.or44.or49.or54, i32 %or59
%.or4.or9.or14.or19.or24.or29.or34.or39.or44.or49.or54.or59.or64 = select i1 %tobool62, i32 %.or4.or9.or14.or19.or24.or29.or34.or39.or44.or49.or54.or59, i32 %or64
%.or4.or9.or14.or19.or24.or29.or34.or39.or44.or49.or54.or59.or64.or69 = select i1 %tobool67, i32 %.or4.or9.or14.or19.or24.or29.or34.or39.or44.or49.or54.or59.or64, i32 %or69
%.or4.or9.or14.or19.or24.or29.or34.or39.or44.or49.or54.or59.or64.or69.or74 = select i1 %tobool72, i32 %.or4.or9.or14.or19.or24.or29.or34.or39.or44.or49.or54.or59.or64.or69, i32 %or74

[spatel]

The code change looks obviously correct, so LGTM.

I'd suggest regenerating the checks in the tests using utils/update_test_checks.py. We really don't want those to be affected by cosmetic name changes, right?

lib/Transforms/Utils/SimplifyCFG.cpp

static bool SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB,
   // Insert a select of the value of the speculated store.
   if (SpeculatedStoreValue) {
     IRBuilder<NoFolder> Builder(BI);
     Value *TrueV = SpeculatedStore->getValueOperand();
     Value *FalseV = SpeculatedStoreValue;
     if (Invert)
       std::swap(TrueV, FalseV);
     Value *S = Builder.CreateSelect(
-        BrCond, TrueV, FalseV, TrueV->getName() + "." + FalseV->getName(), BI);
+        BrCond, TrueV, FalseV, "spec.store.select", BI);
     SpeculatedStore->setOperand(0, S);
     SpeculatedStore->applyMergedLocation(BI->getDebugLoc(),
                                          SpeculatedStore->getDebugLoc());
   }
 
   // Metadata can be dependent on the condition we are hoisting above.
   // Conservatively strip all metadata on the instruction.
   for (auto &I : *ThenBB)
for (BasicBlock::iterator I = EndBB->begin();
 
     // Create a select whose true value is the speculatively executed value and
     // false value is the preexisting value. Swap them if the branch
     // destinations were inverted.
     Value *TrueV = ThenV, *FalseV = OrigV;
     if (Invert)
       std::swap(TrueV, FalseV);
     Value *V = Builder.CreateSelect(
-        BrCond, TrueV, FalseV, TrueV->getName() + "." + FalseV->getName(), BI);
+        BrCond, TrueV, FalseV, "spec.select", BI);
     PN->setIncomingValue(OrigI, V);
     PN->setIncomingValue(ThenI, V);
   }
 
   ++NumSpeculations;
   return true;
 }
 

test/Transforms/SimplifyCFG/SpeculativeExec.ll

 bb1:            ; preds = %entry
   %tmp2 = icmp sgt i32 %c, 1
   br i1 %tmp2, label %bb2, label %bb3
 ; CHECK: bb1:
 ; CHECK-NEXT: icmp sgt i32 %c, 1
 ; CHECK-NEXT: load i32*, i32** %ptr3
 ; CHECK-NOT: dereferenceable
 ; CHECK-NEXT: select i1 %tmp2, i32* %tmp3, i32* %ptr2
-; CHECK-NEXT: ret i32* %tmp3.ptr2
+; CHECK-NEXT: ret i32* %spec.select
 
 bb2:    ; preds = bb1
         %tmp3 = load i32*, i32** %ptr3, !dereferenceable !{i64 10}
   br label %bb3
 
 bb3:    ; preds = %bb2, %entry
   %tmp4 = phi i32* [ %ptr1, %entry ], [ %ptr2, %bb1 ], [ %tmp3, %bb2 ]
   ret i32* %tmp4
 }

test/Transforms/SimplifyCFG/guards.ll

right:
   ret i32 %val.right
 
 ; CHECK: guard_block:
 ; CHECK-NEXT:   call void (i1, ...) @llvm.experimental.guard(i1 false) [ "deopt"() ]
 ; CHECK-NEXT:  unreachable
 
 ; CHECK: merge_block:
 ; CHECK-NEXT:  %c1 = load volatile i1, i1* %c
-; CHECK-NEXT:  [[VAL:%[^ ]]] = select i1 %c1, i32 50, i32 100
+; CHECK-NEXT:  [[VAL:%[^ ]+]] = select i1 %c1, i32 50, i32 100
 ; CHECK-NEXT:  ret i32 [[VAL]]
 }

test/Transforms/SimplifyCFG/merge-cond-stores-2.ll

 ; RUN: opt -S < %s -simplifycfg -simplifycfg-merge-cond-stores=true -simplifycfg-merge-cond-stores-aggressively=false -phi-node-folding-threshold=2 | FileCheck %s
 
 target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
 target triple = "armv7--linux-gnueabihf"
 
 ; This is a bit reversal that has been run through the early optimizer (-mem2reg -gvn -instcombine).
 ; There should be no additional PHIs created at all. The store should be on its own in a predicated
 ; block and there should be no PHIs.
 
 ; CHECK-LABEL: @f
 ; Exactly 15 phis, as there are 15 in the original test case.
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK: select
-; CHECK-NOT: select
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK: select i1
+; CHECK-NOT: select i1
 ; CHECK: br i1 {{.*}}, label %[[L:.*]], label %[[R:.*]]
 ; CHECK: [[L]]: ; preds =
 ; CHECK-NEXT: store
 ; CHECK-NEXT: br label %[[R]]
 ; CHECK: [[R]]: ; preds =
 ; CHECK-NEXT: ret i32 0
 
 define i32 @f(i32* %b) {

test/Transforms/SimplifyCFG/multiple-phis.ll

 ; RUN: opt -simplifycfg -keep-loops=false -S < %s | FileCheck %s
 
 ; It's not worthwhile to if-convert one of the phi nodes and leave
 ; the other behind, because that still requires a branch. If
 ; SimplifyCFG if-converts one of the phis, it should do both.
 
-; CHECK:      %div.high.addr.0 = select i1 %cmp1, i32 %div, i32 %high.addr.0
-; CHECK-NEXT: %low.0.add2 = select i1 %cmp1, i32 %low.0, i32 %add2
+; CHECK:      %spec.select = select i1 %cmp1, i32 %div, i32 %high.addr.0
+; CHECK-NEXT: %spec.select1 = select i1 %cmp1, i32 %low.0, i32 %add2
 ; CHECK-NEXT: br label %while.cond
 
 define i32 @upper_bound(i32* %r, i32 %high, i32 %k) nounwind {
 entry:
   br label %while.cond
 
 while.cond:                                       ; preds = %if.then, %if.else, %entry
   %high.addr.0 = phi i32 [ %high, %entry ], [ %div, %if.then ], [ %high.addr.0, %if.else ]

test/Transforms/SimplifyCFG/select-gep.ll

if.then:
   %incdec.ptr = getelementptr %ST, %ST* %x, i32 0, i32 1
   br label %if.end
 
 if.end:
   %x.addr = phi i8* [ %incdec.ptr, %if.then ], [ %y, %entry ]
   ret i8* %x.addr
 
 ; CHECK-LABEL: @test1(
-; CHECK: %incdec.ptr.y = select i1 %cmp, i8* %incdec.ptr, i8* %y
-; CHECK: ret i8* %incdec.ptr.y
+; CHECK: %spec.select = select i1 %cmp, i8* %incdec.ptr, i8* %y
+; CHECK: ret i8* %spec.select
 }