- User Since
- May 25 2016, 6:41 AM (185 w, 2 h)
Oct 4 2019
I'll abandon this revision, as the suggestion didn't find community approval. I might do lesser things in this area, but if so, that'll be new patches.
Sep 27 2019
Sep 25 2019
Seems irrelevant now e.g. due to the new pass manager.
Should have been abandoned a long time ago.
Sep 20 2019
May 9 2019
Added an inBytes() helper method. (We could contemplate putting this in DataLayout instead for more general use.) Adjusted READNONE to READONLY. Improved comments.
May 2 2019
Mar 14 2019
Oct 1 2018
Thanks, Sanjay. Using makeCmpResultType() in this patch for a smaller and nicer diff.
Sep 27 2018
This patch checks icmp types directly. This might be simpler than checking the operand type.
Sep 25 2018
Sep 20 2018
Used an overloaded getter with simpler interface.
Sep 19 2018
Roman Lebedev commented with an elegant way to create varying integer values, so this patch makes use of that. Changed name of patch to reflect that we now handle rather than ignore vector icmps.
Jun 28 2018
Jun 21 2018
Thanks Krzysztof. I don't have commit rights yet, so I'd appreciate help to get this merged to trunk.
Jun 20 2018
Thanks Adrian for your feedback. The new diff includes refactorings to use MachineInstrBuilder construction functions where possible.
Jun 19 2018
May 7 2018
This works for me too and looks good (but again, I don't know the framework very well). Thanks!
Dec 8 2017
Updated according to Bjorns suggestions: added TiedTo initializations and made bool args initialize to false instead of 0.
Dec 1 2017
The testcase could fail on other targets, since the label names could become different. Therefore, the checks were relaxed, since the test really only needs to verify that it doesn't crash and emits some code.
Nov 30 2017
Sep 14 2017
Reverted the changes in the previous patch, except for the name improvements. I guess we can land this, then. I don't have the privileges yet, so I'm grateful for help with that. :-)
Ah, thanks. Then the fault is in our backend, that we somehow miss to add that imp-use in some circumstances. My previous patch is probably better, then. I'll reupload that with Matthias suggested name changes. :-)
I found and fixed the reason our randomized tests didn't run cleanly. It was essentially that stepBackwards removed defined regs from the set even if the instruction was predicated. That meant a dominating def could be marked dead, which is wrong. The reason I found this is that I use the new recompute function also in a later stage in our out-of-tree backend, and that went bad when I started to add faulty deads. If I understand it correctly, this is a bug in the current implementation of LivePhysRegs.
Sep 13 2017
This new implementation doesn't run cleanly with our extended test suite. I have to look a bit more into it.
This is a new patch that will handle the issue Krzysztof pointed out. Some more Hexagon tests affected. Makes me wonder: Is it ok to say that a predicated instruction's use of a register is killed if it is defined by the same instruction?
Sep 11 2017
Thanks for the reviews, everybody! Here I'm submitting an updated patch based on Matthias' suggestions.
Sep 8 2017
Aug 3 2017
Hmm, sorry, I just realized that the check I introduced in the second patch made it unnecessary to set the subregister conditionally, it could just as well be set unconditionally to zero. So I'm doing this in this update.
Made sure that we don't replace register if the subregister is different.
Krzysztof, thanks, I think it actually would replace it (though it would have before my patch too). I'll upload a patch that'll prevent it.
Aug 2 2017
Jul 3 2017
Added a MIR test for the check. Thanks to Quentin for input.
Jun 28 2017
Ping. How about it? Could we land this patch?
Jun 21 2017
Removed the check that tied physregs and virtregs aren't mixed. Removed changes in a test that had such mixes.
Jun 20 2017
This fix is still relevant and would be nice to land, but 3 out of 4 related TR have gone into hiding/are non-reproducible after AssertingValueHandle functionality was changed. 27697 is still reproducible.
Missed adding llvm-commits as subscriber initially. Adding this note as a ping to the mailing list.
Oct 10 2016
Pinging Mehdi Amini. :-)
Oct 3 2016
Good that this isn't forgotten. :-) It's Monday, so I'm sending another friendly ping.
Sep 12 2016
Monday, so I'm pinging. :-)
Aug 16 2016
So, I'm back in business. :-) Mehdi, what say you?
Jul 8 2016
Whether this is the the right fix or band-aid is a fair question. :-) The problematic operation seems to be the transitive update of LastUser, and that's what I'm restraining in a minimal way to retain consistency with preservation info. OTOH, I don't understand/like the transitive update at all, but perhaps you understand its implications and what, if anything, would need to be done if it were to be removed entirely? There are other possibilities as well but I guess those would require more fundamental redesign.
Jun 23 2016
Thank you to Mehdi Amini for good review comments. I've updated the diff accordingly: Improved description, changed to a more appropriate container type for the book-keeping and also I added a C++ unittest.
Jun 17 2016
Jun 16 2016
Jun 8 2016
Corrected test case in accordance to the review comments from Michael Zolotukhin. (Thanks Michael!)
May 27 2016
Thanks! This is my first contribution and I don't have commit rights. Could you help me commit, please?