[clang] [llvm] [ConstantFold] Drop gep of gep fold entirely (PR #95126)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 12 08:47:25 PDT 2024
agozillon wrote:
Hey, @jplehr notified me of this issue (thank you), I've had a little look at it and it's related to an issue this PR fixes: https://github.com/llvm/llvm-project/pull/94541 which you are currently a reviewer of ironically (as I use and alter slightly a piece of functionality you ported, so thank you very much for that)! A slightly funny convergence there :-)
Currently when generating the kernel we try to replace uses of values with their appropriate kernel input argument, in certain cases these are Constant's which poses an issue as we can't replace them with the Input arguments which are themselves not Constant I believe. So we can only replace instructions, to do so we transform the Constants to Instructions where necessary, the current function that's in place (a small naive function from myself) will do this within the kernel to a small extent, it doesn't handle nesting's very well for example, and apparently IR this patch will produce in the test cases that are failing. However, the PR I have opened above uses the more robust and comprehensive piece of functionality you made/ported via the function `convertUsersOfConstantsToInstructions` which solves the issues encountered when this PR is landed alongside some other issues I encountered recently.
So two ways forward I can see, depending on how urgent/annoying it is for this PR to be stalled:
1. We disable the offending tests that are breaking the buildbot until the PR that will fix them lands, which will allow you to commit this as soon as they're disabled without angering the buildbot
2. We hold off landing this until the PR that fixes the issues in the tests lands, I am unfortunately not sure how long this will take as it's been up for a while with a lack of reviewers (having a review from you @nikic would be excellent as well for the minor additions to the functionality you added), although, I can likely chase some people up to give it a look over as it's urgent.
I don't mind which route we go down, if we opt to disable the tests, I can land a commit shortly to do so, unless either of you @nikic @jplehr would prefer to do so :-)
https://github.com/llvm/llvm-project/pull/95126
More information about the llvm-commits
mailing list