User Details
- User Since
- May 20 2016, 3:15 AM (349 w, 7 h)
Sep 9 2019
Aug 7 2019
Aug 5 2019
Thanks for the suggestion, Sanjoy. Please, review the updated patch.
Aug 1 2019
I am afraid implementing a test through friendship would be pretty cumbersome.
ScalarEvolutionsTest performs checks inside lambdas, while the friendship does
not propagate inside lambdas.
Jul 30 2019
Jan 9 2018
Jan 4 2018
Dec 7 2017
Oct 20 2017
Oct 19 2017
Oct 18 2017
Oct 17 2017
Oct 16 2017
Sep 28 2017
Seems that it has been just fixed by rL314424 :-)
Abandoning this review.
Sep 19 2017
Sep 18 2017
Aug 10 2017
Aug 8 2017
committed r310359
If the patch is accepted, it should also be merged into 5.0, I believe.
Aug 4 2017
Jul 10 2017
Jun 30 2017
Jun 29 2017
Jun 28 2017
Jun 9 2017
Jun 8 2017
May 29 2017
May 9 2017
Apr 18 2017
Apr 2 2017
Mar 26 2017
Done.
Mar 24 2017
Mar 5 2017
And since the set of middle-end optimizations and their parameters are target-specific, I would like to have this test X86-specific. It is not an automatically generated test, it has only one CHECK statement, and therefore it is not a problem to have it as an llc test.
If we we really wanted to run the full pipeline, I don't see why we couldn't simply run opt -O3 | opt -codegenprepare. Can you think of a specific bug in this pass that would not be caught by this that would be caught by the lit test?
Changed the PHI heuristic as planned.
Mar 4 2017
That wouldn't make much sense. The recognized patterns are already checked with simpler tests I have put into bypass-slow-div-special-cases.ll. The purpose of this particular file is to make sure that the patterns we recognize are indeed the patterns produced by the middle-end at the maximal optimization level (I have removed the explicit unroll-count option).
It sounds like there's an implicit "on this particular testcase" at the end of this sentence. "We are checking that we recognize the patterns produced by the middle-end at -O3 *on this particular testcase*."
Mar 2 2017
Superseded by D29897.
Mar 1 2017
Moved most of the tests into CodeGenPrepare/NVPTX and changed RUN line in bypass-slow-division-fnv.ll.
Feb 26 2017
Ok. I see your point. Indeed, for example, there's no need to run the whole X86 code generator to check that a division bypassing is disabled if one of the operands doesn't fit into BypassType. So I have moved the tests introduced by this patch into Transforms/CodeGenPrepare/NVPTX/bypass-slow-div-special-cases.ll. However, I still believe that we should keep a few existing tests in CodeGen/X86/bypass-slow-division-64.ll to check that when bypassing fires, it produces a good X86 code.
That's indeed somewhat frustrating... As I understand, now you all believe it's better to drop all the tests from test/CodeGen/X86/bypass-slow-division-32.ll and test/CodeGen/X86/bypass-slow-division-64.ll and replace them with a few target-independent tests.
Feb 23 2017
The patch has been rebased onto D29897.
Well, it might make sense to write opt tests here... But we already have got 3 files with 18 llc tests for division bypassing. Moreover, while working on this patchset, the tests were thoroughly reviewed, it was insisted that the test should be written this way, I had to make a number of commits specifically to modify the tests. So, I don't think it would be a good idea to throw away all the work that has been done and re-write the tests from scratch now.
Feb 22 2017
As for tests, it was discussed in D28196 that it's better to have them that detailed. I agree with you that they are overly fragile and check for a lot of actually insignificant details. But on the other hand, this approach also has a number of advantages: tests indeed guarantee that the compiler generates correct code for the testcase, the tests are easy to create and update, diffs in tests induced by patches are explicit and easy to review.