- User Since
- Apr 14 2013, 11:48 AM (327 w, 16 h)
Fri, Jul 19
I've been thinking about the getStrictFPOperationAction issue. I believe this function makes no sense at all and really should go away. Instead, the strict opcodes should use their own operation actions just like everything else. In current code, those should now be set up in a reasonable manner: they all default to Expand, unless the target overrides them (to whatever makes sense).
Looks like this patch was reverted on mainline now.
Thu, Jul 18
I tried to run it on my Intel system for comparison, and the test case is failing there too:
Just a quick heads-up: the new test case is failing on SystemZ. I'm not quite sure why, but the build bots show this error:
Tue, Jul 16
Just to clarify one thing: even the current implementation, before this patch, does not guarantee the relative order of FP instructions and memory instructions is unchanged. So even the current implementation may perform the reschedule your comment mentions. This patch would add the additional option of also changing the relative order of the two strict_fmul operations.
Mon, Jul 15
Ping? It would be good to get this in LLVM 9 ...
Fri, Jul 12
I agree it's a bit hacky, but I don't see any better solution either. In any case, the change is not wrong -- everything rejected should be rejected. Given that this fixes a real problem, I'm fine with the patch. LGTM.
Thanks for the quick revert! The new patch looks reasonable to me.
Thu, Jul 11
This commit broke codegen on SystemZ, we're now failing both the test-suite and the multistage bootstrap. Unfortunately this wasn't detected earlier since the SystemZ build bot was down.
Tue, Jul 9
This scheduler generally only operates on single basic blocks, so it would not move anything outside of a test.
Fri, Jul 5
Unfortunately, I think this gets the semantics wrong, because "carry" and "borrow" use *different* CC values on SystemZ.
Fri, Jun 28
Thu, Jun 27
Wed, Jun 26
This seems to have broken test-suite build bots?
Mon, Jun 24
Jun 19 2019
I'm now getting test suite failures like this:
Jun 18 2019
Jun 9 2019
Jun 7 2019
At this point, this looks good to me. Thanks!
Jun 6 2019
Jun 5 2019
Addressed review comments and updated to mainline changes.
Jun 4 2019
Looking mostly good now, thanks! Just a few remaining questions/comments inline ...
Jun 3 2019
I'm a bit confused about the mapping logic. For the case of e.g. ADD, we today have
AR ---> gets mapped to A by getMemOpcode
ARK --> no mapping via getMemOpcode
May 31 2019
I think this is good for now. We can work on further improving folded reloads as a follow-on. LGTM.
I agree this still checks the same thing, so the patch LGTM.
May 29 2019
As of r361920, the SystemZ build bots are green again. Thanks, Eric!
May 27 2019
Looks like this test is failing on SystemZ since it was added, making all our build bots red:
May 16 2019
May 13 2019
Updated to address issues raised during the review:
Apr 29 2019
Apr 26 2019
Sorry if I was not clear in my description, but what I meant to illustrate is that the RA allocates the VRegs in the order I listed them. So first %21 is allocated $r2d, and then the next VReg assigned is %12, and then %13, %14, ..., %20, %21.
> %12 -> $r0d // does not see that $r2d would be better
Apr 25 2019
It looks like this new test case fails on big-endian systems, like this:
Apr 24 2019
Apr 23 2019
Apr 22 2019
Those test case changes represent an actual improvement here, so this looks good. Thanks!
Apr 19 2019
This looks really promising, in particular the reduction in spills and copies. Can you check that this also addresses the problem described here: https://reviews.llvm.org/D22011 ?
Apr 16 2019
Apr 15 2019
Not a full review yet, but a couple of comments (in the test case) where the resulting code could be simplified.
Apr 10 2019
On SystemZ, there is no dependency from InstPrinter to MCTargetDesc (or anything else in the target) as far as I can see. That means we don't have an issue, right?
Apr 4 2019
Apr 3 2019
Mar 29 2019
The SystemZ test case change is fine with me, thanks!
Mar 17 2019
Mar 4 2019
Feb 27 2019
Feb 26 2019
OK, this version LGTM. Thanks!
Feb 25 2019
Feb 21 2019
This looks generally good to me. Some additional options to possibly make the code simpler occurred to me:
The SystemZ part LGTM, thanks.
Feb 20 2019
Feb 19 2019
Thanks, this should now cover all intrinsics with immediate arguments. Some additional comments inline (mostly cosmetic, with one real fix for int_s390_vfisb).
Feb 18 2019
Could we actually handle FP128 as well with a present FeatureVectorEnhancements1?
I'm not quite sure I understand the logic why some intrinsics that require immediate arguments are marked with ImmArg, but others are not?
Feb 14 2019
Ah, good catch! LGTM.
Feb 13 2019
Hmm. Actually, I'm now wondering why we need to reject anything in the first place. Can't we improve isFPImmLegal to accept *anything* that can be constructed via any of the vector instructions (VGBM, VGM, VREPI)?
Feb 12 2019
Yes, I think this version is better.
Feb 11 2019
It seems you're assuming VGM is always available. I guess there needs to be a check for hasVector() somewhere.