User Details
- User Since
- May 29 2017, 8:02 AM (304 w, 5 d)
Yesterday
Overall I think that this looks fine to me as well.
I had a couple of minor comments and you may decide that you don't need to do either one so if that's the case just mention why in a comment and I will approve the patch.
Wed, Mar 29
Updated test case to include another thread local value.
Updated the code to include all of the thread local flags.
Fri, Mar 24
Overall I think that this patch makes sense to me. A few minor things:
- I think that there are some changes that are nice improvements but aren't really part of the kill flags fix. This is already a fairly large patch and removing these little fixes might make the patch clearer and easier to read. They can be made before or after with a (or several) NFC change(s).
- I know this is an older patch and I apologize for taking so long to look at it. When I tried to rebase this patch to the Top of main I realized that I had to add a couple more addRegToUpdate in order to get it working. This isn't an issue with the patch it's just that new changes to PPCMIPeephole will require additions or addRegToUpdate.
- Is there any way to provide an error or warning for the future when more peephole optimizations are added and for whatever reason the developer forgets to call addRegToUpdate? There may not be a way to do this and that's fine. I'm more curious.
Thu, Mar 23
I realize that a lot of test cases have changed but it would be good to add the test case that is specific to this opportunity where the copy is removed.
Mon, Mar 20
Split the global TOC entries into Local and External Linkage.
Fri, Mar 17
Fixed a couple of spacing issues that I missed when I put the patch up.
Mar 2 2023
@luismarques
Hi,
It looks like your patch broke our build bot on AIX:
https://lab.llvm.org/buildbot/#/builders/214/builds/6115
Feb 24 2023
Rebased to top of trunk.
Feb 15 2023
@chelfi
It looks like this changeset has broken one of the bots on PowerPC.
https://lab.llvm.org/buildbot/#/builders/57
Feb 14 2023
Feb 13 2023
Added a comment to the test case.
Rebased to top of trunk.
Jan 27 2023
Overall I think this patch looks great.
Jan 19 2023
Removed one line of the test that was not required.
Jan 4 2023
Rebased to top of main branch.
Dec 21 2022
Dec 20 2022
LGTM
Dec 16 2022
Dec 9 2022
I've made the test X86 only to allow time for this to be investigated.
34a3259fab86aaa1a20224e08849775f3593e6a3
This should help bring the bots back to green in the meantime.
It looks like the test test/DebugInfo/assignment-tracking/X86/remove-undef-fragment.ll fails on multiple PPC bots.
https://lab.llvm.org/buildbot/#/builders/214/builds/4756
https://lab.llvm.org/buildbot/#/builders/231/builds/6006
https://lab.llvm.org/buildbot/#/builders/121/builds/26032
Dec 2 2022
Silly question:
Can we now get rid of CC_PPC64_ELF_FIS completely?
Dec 1 2022
I would like to address the following comment because it is not at all obvious why I decided not to go with this approach.
This looks wrong. This will produce a pair of 32-bit single precision values in the FP portion of the VSX register. What you want is a 64-bit double precision value in each doubleword. Don't forget that single precision values on PPC are in registers as double precison but rounded to single precision (i.e. a double precision representation of a single precision value). I think you should use scalar instructions for scalar values (xscvsxdsp, xscvsxddp).
Removed unnecessary whitespace change.
Fixed the patch and a number of test cases.
Nov 30 2022
I'm going to commandeer this patch and rebase it to see if I can revive it.
There are some tests failing with this patch. I'm looking into them now...
Moved the constant materialization to the td file.
Nov 29 2022
Thank you for fixing this!
LGTM.
Nov 28 2022
Thank you for fixing this.
I do have a couple of comments related to the fix.
Forgot to do a clang-format.
Updated patch with that.
Nov 22 2022
Rebased to top of main branch.
Nov 21 2022
Thank you for fixing that last test!
LGTM.
Nov 10 2022
Nov 8 2022
Sorry, I know that I had approved this before but it seems that the test p10-splatImm32-undef.ll starts failing with this patch.
It may just be that the test needs to be updated but please make sure that is all it is.
I think this looks good.
Thank you for addressing the comments!
Nov 3 2022
Fixed some spacing and a couple of comments.
Other than a very minor nit I think this LGTM.
Nov 2 2022
Moved the include location for PPCInstrFutureMMA.td.
Rebased patch to top of main branch.
Oct 28 2022
Thank you for your patience and sorry it took me so long to get to this.
Oct 27 2022
Rebased patch to top of main and addressed a couple of nits.
Oct 25 2022
Oct 20 2022
nit: Fixed some spacing.
Oct 18 2022
Oct 14 2022
Oct 12 2022
Oct 6 2022
Oct 5 2022
Oct 4 2022
Rebased to top of main branch.
Sep 22 2022
Sep 19 2022
Thank you both for looking at this patch.
Looking at the comments I feel like I made an incorrect assumption about NULL pointers in this case and I'm just going to abandon this patch.
Sep 13 2022
Sorry that this took a few days.
Removed the code from LICM.
Added a test for the loop strength reduction. The test shows simpler code when
the guard for the NULL is added. However, there may be other tests where the
reverse is true.
Sep 8 2022
This is better than what I initially tried to do in D133426.
LGTM