[PATCH] D73919: [InstCombine] Use replaceOperand() in more places

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

Seems like good cleanup. I suspect the implementation differences are based on historical prefs, but it's possible there's some compile-time perf difference in new instruction vs. recycling. I doubt that would rise above the noise, but to be safe, I'd split this up:

1. Make the select swapValues() changes - should be NFC.
2. Change the multi-operand set patterns to Create*.
3. Use replaceOperand().



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1383-1385
+    replaceOperand(I, 0, A);
+    replaceOperand(I, 1, B);
     return &I;
----------------
This is different than the CreateXor() changes below because of overflow flag propagation? If yes, it's worth a code comment.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2402
     Cond->setPredicate(CmpInst::getInversePredicate(Pred));
-    SI.setOperand(1, FalseVal);
-    SI.setOperand(2, TrueVal);
+    SI.swapValues();
     SI.swapProfMetadata();
----------------
Better to make this an independent patch.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2781
+    replaceOperand(SI, 0, NotCond);
+    SI.swapValues();
     SI.swapProfMetadata();
----------------
Similar to above - pre-commit the swapValues() diff? Or maybe better to create a new instruction here similar to the earlier binop changes?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73919/new/

https://reviews.llvm.org/D73919