[PATCH] D84547: [ConstraintElimination] Add constraint elimination pass.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 1 12:30:00 PDT 2020


fhahn updated this revision to Diff 289249.
fhahn added a comment.



In D84547#2242280 <https://reviews.llvm.org/D84547#2242280>, @spatel wrote:

> I'm not familiar with the underying ConstraintSystem (Fourier–Motzkin elimination), so if anyone else has experience with it and can chime in, that would be nice.

In a way, we delegate most of the logic to a 'black-box', the constraint system, which potentially makes debugging failures a bit more tricky.

It should however be possible to record the constraints and the found solutions and use a different solver (e.g. Z3) to verify the implementation in LLVM produces the correct result. I put up D86966 <https://reviews.llvm.org/D86966>, which adds a nice way to print the system and solution for each check if there is a solution.

It also adds a script which generates a python script that uses Z3's API to feed the same constraints to Z3 and checks if it produces the same result. I use this to verify all queries generated when building the test-suite (MultiSource/SPEC2000/SEPC2006) with LTO and all queries matched.

> Would the overflow concerns mentioned in the description be handled more easily by changing the ConstraintSystem to use APInt?

I am not sure. I think it would boil down replacing the calls to __builtin_mul_overflow & co with APInt's smul_ov & co. Unless I am missing something, I am not sure if that is enough benefit to complicate things by using APInt everywhere.

> Given that this is off-by-default, I don't think this needs detailed review at this stage - we have sufficient optimization motivation, and the cost of keeping it in trunk while making improvements isn't high. Either we'll eventually flip it to default-on because it will be shown to be worth its compile-time cost, or it will be dismissed and removed.

That sounds good to me, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84547

Files:
  llvm/include/llvm/Analysis/ConstraintSystem.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Scalar.h
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Scalar/CMakeLists.txt
  llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
  llvm/lib/Transforms/Scalar/Scalar.cpp
  llvm/test/Transforms/ConstraintElimination/dom.ll
  llvm/test/Transforms/ConstraintElimination/geps.2d.ll
  llvm/test/Transforms/ConstraintElimination/geps.ll
  llvm/test/Transforms/ConstraintElimination/i128.ll
  llvm/test/Transforms/ConstraintElimination/loops.ll
  llvm/test/Transforms/ConstraintElimination/mixed.ll
  llvm/test/Transforms/ConstraintElimination/uge.ll
  llvm/test/Transforms/ConstraintElimination/ugt-ule.ll
  llvm/test/Transforms/ConstraintElimination/ule.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84547.289249.patch
Type: text/x-patch
Size: 34358 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200901/c972550a/attachment.bin>


More information about the llvm-commits mailing list