- User Since
- Jul 4 2017, 1:39 AM (206 w, 2 d)
Wed, Jun 9
fixed nits reported by build bots
Tue, Jun 8
Thu, May 27
Wed, May 26
Philip, thank you for comments. I believe I addressed them all.
Mon, May 24
I'm still working on the next version.
Fri, May 21
I have just committed the patch as
Sorry I forget to add the 'Differential Revision:' line to the comment.
Wed, May 19
I'm ok with the patch and will land it. Lets wait for @DaniilSuchkov's resolution.
May 18 2021
Apr 13 2021
Apr 8 2021
Added 3 test cases with live-in deopt values that crash.
Removed one unrelated comment .
Fixed couple of places with hardcoded 64 const.
Marked the patch as work-in-progress.
Apr 7 2021
Apr 6 2021
I found this constraint accidentally while preparing a test for D100009 where I wanted to check that an arbitrary sized types are ok for spill slot size promotion.
Apr 5 2021
Some tests were moved a separate patch (D99878).
Apr 1 2021
Mar 31 2021
@skatkov, @fedor.sergeev, could you please review. During the long time this features is on the reconstruction we introduced a new bug in the LoopFlatten pass. See the comment https://reviews.llvm.org/D90940#inline-938253.
Mar 29 2021
fixed the nits
just rebased. please review
Mar 25 2021
fixed the test
Dec 27 2020
Dec 25 2020
Dec 23 2020
LGTM if my latest comments are addressed.
Let's give this big work a try.
Dec 18 2020
Landed as https://reviews.llvm.org/rGf0e3d1d6ca8c
Ok. See D93521
Dec 17 2020
Dec 14 2020
Dec 11 2020
Dec 3 2020
Nov 22 2020
Thanks. I'm working closely with @skatkov and believe he have enough expertise to review fully.
The biggest issue with the current design is that we have to send FAM to the initialization of StandardInstrumentations/PreservedCFGCheckerInstrumentation. This looks asymmetric to the other analysis managers and it would be more reasonable to send FAM or all AMs to the instrumentation callbacks as parameters.
Nov 19 2020
Not needed after NFC https://github.com/llvm/llvm-project/commit/02dda1c by @nikic
Nov 18 2020
Nov 17 2020
Only PassManagerTest.FunctionPassCFGCheckerInvalidateAnalysis does fail.
The other two (PassManagerTest.FunctionPassCFGCheckerWrapped and PassManagerTest.FunctionPassCFGChecker) pass.
Nov 16 2020
added a test
Nov 15 2020
agree. See my comments in D91017.
The patch looks good, but I have some concerns.
Nov 12 2020
Nov 10 2020
I could not create a simple test. One more option to create one is to change the usage of BPI/BFI to request analysis from AM. Then the test can print BPI to show the difference. I'm not sure this big change is worth this small patch. It is not clear why JumpThreading uses its own local version of BPI/BFI.
Nov 8 2020
This looks to be a good change towards simplification. I think originally a map over edges was introduced by @atrick back in 2011 in https://github.com/llvm/llvm-project/commit/49371f3f33788 as it could be sparse. But with the commit https://gitlab.azulsystems.com/dev/orca/commit/8138487468e22 we imposed an important restriction on setting outgoing edge probabilities at once for one block. This allowed correctness checks and further improvements like this.
Nov 5 2020
Oct 29 2020
EmitGEPOffset() is used by instcombine and we do not see the redundant add 0, X instruction because it is immediately optimized out in the same iteration. It prevents a simple lit test for this patch. On the other hand you can see the fix in InstCombinerImpl::OptimizePointerDifference() which changes the code to expect X in place of add 0, X and there is a related test (test/Transforms/InstCombine/sub-gep.ll), which I believe is enough for this small improvement.
Keep in mind that EmitGEPOffset() could be used in other custom passes that might not require subsequent instcombine to run.
Oct 27 2020
Oct 12 2020
I'm still reading. The patch is long but I'd like to avoid splitting it into pieces that might result in massive tests changes (back and forth). Now the test changes seem to be compact.
It would be great if someone could join the reviewing efforts.