[PATCH] D158837: [ConstraintElim] Store conditional facts as (Predicate, Op0, Op1).

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:1415
+        dbgs() << "Processing fact to add to the system: "
+               << CmpInst::getPredicateName(Pred) << " ";
+        A->printAsOperand(dbgs());
----------------
nikic wrote:
> I think you can directly `<< Pred` nowadays.
Nice, updated in the committed version, thanks!


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:1418
+        dbgs() << ", ";
+        B->printAsOperand(dbgs());
+        dbgs() << "\n";
----------------
nikic wrote:
> You can pass false to avoid printing the type again.
Nice, updated in the committed version, thanks!


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:1461
+    if (A)
+      AddFact(Pred, A, B);
   }
----------------
nikic wrote:
> I feel like this code would be simpler/cleaner if you didn't share this AddFact call and just did it in both branches above.
Add fact can be used unconditionally here. Also simplified the matching for the assume condition and added an assert that it gets matched.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158837