llvm-reduce: Fix opcode reduction leaving behind dead instructions

[arsenm]

Summary

ce3c3cb2912425bb4367bfbe9a4c68a6d6f0a04a broke this by
speculatively making transforms before checking shouldKeep.
Originally I tried to roll back changes to the IR, but it's probably
best to not touch it before querying.

[aeubanks]

whoops, sorry I missed this in my patch

the alternative is to not using IRBuilder and not insert the instruction into the module unless the oracle says so, this patch is a little messier code wise but less work at runtime

[arsenm]

3e6f7ab867ac8c36f0c8a91e7fa1608742702681

llvm/test/tools/llvm-reduce/reduce-opcodes.ll

 ; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=opcodes --test FileCheck --test-arg %s --test-arg --input-file %s -o %t
 ; RUN: FileCheck -check-prefixes=CHECK,RESULT %s < %t
 
 ; CHECK-LABEL: @fdiv_fast(
-; RESULT: %op = fmul fast float %a, %b, !dbg !7, !fpmath !13
+; RESULT-NEXT: %op = fmul fast float %a, %b, !dbg !7, !fpmath !13
+; RESULT-NEXT: ret float %op
 define float @fdiv_fast(float %a, float %b) {
   %op = fdiv fast float %a, %b, !dbg !7, !fpmath !13
   ret float %op
 }
 
 ; CHECK-LABEL: @frem_nnan(
-; RESULT: %op = fmul nnan float %a, %b, !dbg !7, !fpmath !13
+; RESULT-NEXT: %op = fmul nnan float %a, %b, !dbg !7, !fpmath !13
+; RESULT-NEXT: ret float %op
 define float @frem_nnan(float %a, float %b) {
   %op = frem nnan float %a, %b, !dbg !7, !fpmath !13
   ret float %op
 }
 
 ; CHECK-LABEL: @udiv(
-; RESULT: %op = mul i32 %a, %b, !dbg !7
+; RESULT-NEXT: %op = mul i32 %a, %b, !dbg !7
+; RESULT-NEXT: ret i32 %op
 define i32 @udiv(i32 %a, i32 %b) {
   %op = udiv i32 %a, %b, !dbg !7
   ret i32 %op
 }
 
 ; CHECK-LABEL: @udiv_vec(
-; RESULT: %op = mul <2 x i32> %a, %b, !dbg !7
+; RESULT-NEXT: %op = mul <2 x i32> %a, %b, !dbg !7
+; RESULT-NEXT: ret <2 x i32> %op
 define <2 x i32> @udiv_vec(<2 x i32> %a, <2 x i32> %b) {
   %op = udiv <2 x i32> %a, %b, !dbg !7
   ret <2 x i32> %op
 }
 
 ; CHECK-LABEL: @sdiv(
-; RESULT: %op = mul i32 %a, %b{{$}}
+; RESULT-NEXT: %op = mul i32 %a, %b{{$}}
+; RESULT-NEXT: ret i32 %op
 define i32 @sdiv(i32 %a, i32 %b) {
   %op = sdiv i32 %a, %b
   ret i32 %op
 }
 
 ; CHECK-LABEL: @sdiv_exact(
-; RESULT: %op = mul i32 %a, %b, !dbg !7
+; RESULT-NEXT: %op = mul i32 %a, %b, !dbg !7
+; RESULT-NEXT: ret
 define i32 @sdiv_exact(i32 %a, i32 %b) {
   %op = sdiv exact i32 %a, %b, !dbg !7
   ret i32 %op
 }
 
 ; CHECK-LABEL: @urem(
-; RESULT: %op = mul i32 %a, %b, !dbg !7
+; RESULT-NEXT: %op = mul i32 %a, %b, !dbg !7
+; RESULT-NEXT: ret
 define i32 @urem(i32 %a, i32 %b) {
   %op = urem i32 %a, %b, !dbg !7
   ret i32 %op
 }
 
 ; CHECK-LABEL: @srem(
-; RESULT: %op = mul i32 %a, %b, !dbg !7
+; RESULT-NEXT: %op = mul i32 %a, %b, !dbg !7
+; RESULT-NEXT: ret
 define i32 @srem(i32 %a, i32 %b) {
   %op = srem i32 %a, %b, !dbg !7
   ret i32 %op
 }
 
 ; Make sure there's no crash if the IRBuilder decided to constant fold something
 ; CHECK-LABEL: @add_constant_fold(
-; RESULT: %op = add i32 0, 0, !dbg !7
+; RESULT-NEXT: %op = add i32 0, 0, !dbg !7
+; RESULT-NEXT: ret
 define i32 @add_constant_fold() {
   %op = add i32 0, 0, !dbg !7
   ret i32 %op
 }
 
 ; CHECK-LABEL: @add(
-; RESULT: %op = or i32 %a, %b, !dbg !7
+; RESULT-NEXT: %op = or i32 %a, %b, !dbg !7
+; RESULT-NEXT: ret
 define i32 @add(i32 %a, i32 %b) {
   %op = add i32 %a, %b, !dbg !7
   ret i32 %op
 }
 
 ; CHECK-LABEL: @add_nuw(
-; RESULT: %op = or i32 %a, %b, !dbg !7
+; RESULT-NEXT: %op = or i32 %a, %b, !dbg !7
+; RESULT-NEXT: ret
 define i32 @add_nuw(i32 %a, i32 %b) {
   %op = add nuw i32 %a, %b, !dbg !7
   ret i32 %op
 }
 
 ; CHECK-LABEL: @add_nsw(
-; RESULT: %op = or i32 %a, %b, !dbg !7
+; RESULT-NEXT: %op = or i32 %a, %b, !dbg !7
+; RESULT-NEXT: ret
 define i32 @add_nsw(i32 %a, i32 %b) {
   %op = add nsw i32 %a, %b, !dbg !7
   ret i32 %op
 }
 
 ; CHECK-LABEL: @sub_nuw_nsw(
-; RESULT: %op = or i32 %a, %b, !dbg !7
+; RESULT-NEXT: %op = or i32 %a, %b, !dbg !7
+; RESULT-NEXT: ret
 define i32 @sub_nuw_nsw(i32 %a, i32 %b) {
   %op = sub nuw nsw i32 %a, %b, !dbg !7
   ret i32 %op
 }
 
 ; CHECK-LABEL: @workitem_id_y(
-; RESULT: %id = call i32 @llvm.amdgcn.workitem.id.x(), !dbg !7
+; RESULT-NEXT: %id = call i32 @llvm.amdgcn.workitem.id.x(), !dbg !7
+; RESULT-NEXT: ret
 define i32 @workitem_id_y() {
   %id = call i32 @llvm.amdgcn.workitem.id.y(), !dbg !7
   ret i32 %id
 }
 
 ; CHECK-LABEL: @workitem_id_z(
-; RESULT: %id = call i32 @llvm.amdgcn.workitem.id.x(), !dbg !7
+; RESULT-NEXT: %id = call i32 @llvm.amdgcn.workitem.id.x(), !dbg !7
+; RESULT-NEXT: ret
 define i32 @workitem_id_z() {
   %id = call i32 @llvm.amdgcn.workitem.id.z(), !dbg !7
   ret i32 %id
 }
 
 ; CHECK-LABEL: @workgroup_id_y(
-; RESULT: %id = call i32 @llvm.amdgcn.workgroup.id.x(), !dbg !7
+; RESULT-NEXT: %id = call i32 @llvm.amdgcn.workgroup.id.x(), !dbg !7
+; RESULT-NEXT: ret
 define i32 @workgroup_id_y() {
   %id = call i32 @llvm.amdgcn.workgroup.id.y(), !dbg !7
   ret i32 %id
 }
 
 ; CHECK-LABEL: @workgroup_id_z(
-; RESULT: %id = call i32 @llvm.amdgcn.workgroup.id.x(), !dbg !7
+; RESULT-NEXT: %id = call i32 @llvm.amdgcn.workgroup.id.x(), !dbg !7
+; RESULT-NEXT: ret
 define i32 @workgroup_id_z() {
   %id = call i32 @llvm.amdgcn.workgroup.id.z(), !dbg !7
   ret i32 %id
 }
 
 ; CHECK-LABEL: @minnum_nsz(
-; RESULT: %op = fmul nsz float %a, %b, !dbg !7
+; RESULT-NEXT: %op = fmul nsz float %a, %b, !dbg !7
+; RESULT-NEXT: ret
 define float @minnum_nsz(float %a, float %b) {
   %op = call nsz float @llvm.minnum.f32(float %a, float %b), !dbg !7
   ret float %op
 }
 
 ; CHECK-LABEL: @maxnum_nsz(
-; RESULT: %op = fmul nsz float %a, %b, !dbg !7
+; RESULT-NEXT: %op = fmul nsz float %a, %b, !dbg !7
+; RESULT-NEXT: ret
 define float @maxnum_nsz(float %a, float %b) {
   %op = call nsz float @llvm.maxnum.f32(float %a, float %b), !dbg !7
   ret float %op
 }
 
 ; CHECK-LABEL: @minimum_nsz(
-; RESULT: %op = fmul nsz float %a, %b, !dbg !7
+; RESULT-NEXT: %op = fmul nsz float %a, %b, !dbg !7
+; RESULT-NEXT: ret
 define float @minimum_nsz(float %a, float %b) {
   %op = call nsz float @llvm.minimum.f32(float %a, float %b), !dbg !7
   ret float %op
 }
 
 ; CHECK-LABEL: @maximum_nsz(
-; RESULT: %op = fmul nsz float %a, %b, !dbg !7
+; RESULT-NEXT: %op = fmul nsz float %a, %b, !dbg !7
+; RESULT-NEXT: ret
 define float @maximum_nsz(float %a, float %b) {
   %op = call nsz float @llvm.maximum.f32(float %a, float %b), !dbg !7
   ret float %op
 }
 
 ; CHECK-LABEL: @sqrt_ninf(
-; RESULT: %op = fmul ninf float %a, 2.000000e+00, !dbg !7
+; RESULT-NEXT: %op = fmul ninf float %a, 2.000000e+00, !dbg !7
+; RESULT-NEXT: ret
 define float @sqrt_ninf(float %a, float %b) {
   %op = call ninf float @llvm.sqrt.f32(float %a), !dbg !7
   ret float %op
 }
 
 ; CHECK-LABEL: @sqrt_vec(
-; RESULT: %op = fmul <2 x float> %a, <float 2.000000e+00, float 2.000000e+00>, !dbg !7
+; RESULT-NEXT: %op = fmul <2 x float> %a, <float 2.000000e+00, float 2.000000e+00>, !dbg !7
+; RESULT-NEXT: ret
 define <2 x float> @sqrt_vec(<2 x float> %a, <2 x float> %b) {
   %op = call <2 x float> @llvm.sqrt.v2f32(<2 x float> %a), !dbg !7
   ret <2 x float> %op
 }
 
 ; CHECK-LABEL: @div_fixup(
-; RESULT: %op = call float @llvm.fma.f32(float %a, float %b, float %c)
+; RESULT-NEXT: %op = call float @llvm.fma.f32(float %a, float %b, float %c)
+; RESULT-NEXT: ret
 define float @div_fixup(float %a, float %b, float %c) {
   %op = call float @llvm.amdgcn.div.fixup.f32(float %a, float %b, float %c)
   ret float %op
 }
 
 ; CHECK-LABEL: @fma_legacy(
-; RESULT: %op = call float @llvm.fma.f32(float %a, float %b, float %c)
+; RESULT-NEXT: %op = call float @llvm.fma.f32(float %a, float %b, float %c)
+; RESULT-NEXT: ret
 define float @fma_legacy(float %a, float %b, float %c) {
   %op = call float @llvm.amdgcn.fma.legacy(float %a, float %b, float %c)
   ret float %op
 }
 
 ; CHECK-LABEL: @fmul_legacy(
-; RESULT: %op = fmul float %a, %b
+; RESULT-NEXT: %op = fmul float %a, %b
+; RESULT-NEXT: ret
 define float @fmul_legacy(float %a, float %b) {
   %op = call float @llvm.amdgcn.fmul.legacy(float %a, float %b)
   ret float %op
 }
 
 declare i32 @llvm.amdgcn.workitem.id.y()
 declare i32 @llvm.amdgcn.workitem.id.z()
 declare i32 @llvm.amdgcn.workgroup.id.y()

llvm/tools/llvm-reduce/deltas/ReduceOpcodes.cpp

 static Value *replaceIntrinsic(Module &M, IntrinsicInst *II,
                                Intrinsic::ID NewIID,
                                ArrayRef<Type *> Tys = None) {
   Function *NewFunc = Intrinsic::getDeclaration(&M, NewIID, Tys);
   II->setCalledFunction(NewFunc);
   return II;
 }
 
-static Value *reduceIntrinsic(Module &M, IntrinsicInst *II) {
+static Value *reduceIntrinsic(Oracle &O, Module &M, IntrinsicInst *II) {
   IRBuilder<> B(II);
   switch (II->getIntrinsicID()) {
   case Intrinsic::sqrt:
+    if (O.shouldKeep())
+      return nullptr;
+
     return B.CreateFMul(II->getArgOperand(0),
                         ConstantFP::get(II->getType(), 2.0));
   case Intrinsic::minnum:
   case Intrinsic::maxnum:
   case Intrinsic::minimum:
   case Intrinsic::maximum:
   case Intrinsic::amdgcn_fmul_legacy:
+    if (O.shouldKeep())
+      return nullptr;
     return B.CreateFMul(II->getArgOperand(0), II->getArgOperand(1));
   case Intrinsic::amdgcn_workitem_id_y:
   case Intrinsic::amdgcn_workitem_id_z:
+    if (O.shouldKeep())
+      return nullptr;
     return replaceIntrinsic(M, II, Intrinsic::amdgcn_workitem_id_x);
   case Intrinsic::amdgcn_workgroup_id_y:
   case Intrinsic::amdgcn_workgroup_id_z:
+    if (O.shouldKeep())
+      return nullptr;
     return replaceIntrinsic(M, II, Intrinsic::amdgcn_workgroup_id_x);
   case Intrinsic::amdgcn_div_fixup:
   case Intrinsic::amdgcn_fma_legacy:
+    if (O.shouldKeep())
+      return nullptr;
     return replaceIntrinsic(M, II, Intrinsic::fma, {II->getType()});
   default:
     return nullptr;
   }
 }
 
-static Value *reduceInstruction(Module &M, Instruction &I) {
+static Value *reduceInstruction(Oracle &O, Module &M, Instruction &I) {
   IRBuilder<> B(&I);
   switch (I.getOpcode()) {
   case Instruction::FDiv:
   case Instruction::FRem:
+    if (O.shouldKeep())
+      return nullptr;
+
     // Divisions tends to codegen into a long sequence or a library call.
     return B.CreateFMul(I.getOperand(0), I.getOperand(1));
   case Instruction::UDiv:
   case Instruction::SDiv:
   case Instruction::URem:
   case Instruction::SRem:
+    if (O.shouldKeep())
+      return nullptr;
+
     // Divisions tends to codegen into a long sequence or a library call.
     return B.CreateMul(I.getOperand(0), I.getOperand(1));
   case Instruction::Add:
   case Instruction::Sub: {
+    if (O.shouldKeep())
+      return nullptr;
+
     // Add/sub are more likely codegen to instructions with carry out side
     // effects.
     return B.CreateOr(I.getOperand(0), I.getOperand(1));
   }
   case Instruction::Call: {
     if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(&I))
-      return reduceIntrinsic(M, II);
+      return reduceIntrinsic(O, M, II);
 
     return nullptr;
   }
   default:
     return nullptr;
   }
 
   return nullptr;
 }
 
 static void replaceOpcodesInModule(Oracle &O, Module &Mod) {
   for (Function &F : Mod) {
     for (BasicBlock &BB : F)
       for (Instruction &I : make_early_inc_range(BB)) {
-
         Instruction *Replacement =
-            dyn_cast_or_null<Instruction>(reduceInstruction(Mod, I));
+            dyn_cast_or_null<Instruction>(reduceInstruction(O, Mod, I));
         if (Replacement && Replacement != &I) {
-          if (O.shouldKeep())
-            continue;
-
           if (isa<FPMathOperator>(Replacement))
             Replacement->copyFastMathFlags(&I);
 
           Replacement->copyIRFlags(&I);
           Replacement->copyMetadata(I);
           Replacement->takeName(&I);
           I.replaceAllUsesWith(Replacement);
           I.eraseFromParent();
         }
       }
   }
 }
 
 void llvm::reduceOpcodesDeltaPass(TestRunner &Test) {
   runDeltaPass(Test, replaceOpcodesInModule, "Reducing Opcodes");
 }