Currently working at SiFive.
- User Since
- Jan 5 2016, 9:21 AM (240 w, 8 h)
hasSideEffects may imply isNotDuplicable, especially when rematerializing, but the latter prevents duplication more extensively in the middle end (e.g., tail end duplication).
Wed, Aug 5
Thu, Jul 30
I'm not familiar with the AMDGPU target, but the changes in these new tests seem harmless, except in CodeGen/AMDGPU/stack-realign.ll, where an instruction disappeared. Again, I'd defer to someone who works in this backend, perhaps @arsenm, to chime in.
Tue, Jul 28
Fri, Jul 24
Wed, Jul 22
Tue, Jul 21
Other than that, it seems sensible.
The AMDGPU test change likely means nothing, but it'd be good if someone who maintains or work that target would ok it. I suggest giving it another week or so. Otherwise, it LGTM.
Mon, Jul 20
Fri, Jul 17
Just a couple of nits, but otherwise it LGTM.
Mon, Jul 13
Jul 10 2020
Jul 9 2020
Jul 6 2020
Jul 2 2020
Jun 30 2020
Jun 29 2020
It LGTM after D80802.
Jun 25 2020
Jun 19 2020
It LGTM, but it's better to wait for an OK from a couple or so of other reviewers to chime in.
Jun 15 2020
Jun 4 2020
Again, the clang part should be split in another patch and be made a child of D81188.
It looks pretty GTM. At this point, I'd be fine with accepting this patch as the major issues seem to have already been addressed. Should there be any other minor issue, it could be addressed later.
May 8 2020
May 6 2020
pow(x, -∞) → ∞ if |x| < 1 or 0 if |x| > 1
pow(x, +∞) → 0 if |x| < 1 or ∞ if |x| > 1
exp2(log2(x) * -∞) → exp2(±∞) → ∞ if x < 2 or 0 if x > 2
exp2(log2(x) * +∞) → exp2(±∞) → 0 if x < 2 or ∞ if x > 2
pow(∞, y) → 0 if y < 0 or ∞ if y > 0
exp2(log2(∞) * y) → exp2(∞ * y) → exp2(∞) → 0 if y < 0 or ∞ if y > 0
May 4 2020
Mar 10 2020
It'd be nice if a single predicate wouldn't need all_of, but I don't feel strongly about it.
Feb 18 2020
Would it be possible to add a test case for AArch64?
Feb 14 2020
Feb 13 2020
It's a logical addition to the semantics of features. Though at the moment RV is probably the only user of this addition, the eventual patch implementing it should not include the RV specific changes, which would be better in a separate patch.
Feb 12 2020
Feb 4 2020
Shouldn't most instructions have the vtype register in Uses? Shouldn't most FP instructions have fflags in Defs?
Jan 31 2020
Please, prefix the title with [RISCV].
Jan 29 2020
As @lenary said, this must be enabled only for those sub targets that actually fuse these instrs. Also, when using the macro fusion pass, you should also enable the post RA pass. Otherwise, it's less effective.
Jan 24 2020
Jan 23 2020
Jan 21 2020
Jan 20 2020
It LGTM, but let's wait till tomorrow, when the patch will be 3 business days old, for any input.
Jan 16 2020
Except for a handful of gnats, this is really close to LGTM.
Jan 13 2020
Jan 10 2020
It seems to me that all remarks have already been addressed. Is there anything holding this patch? For it pretty much LGTM.
Overall, it looks nearly good enough. I don't know how the scheduler will behave is interesting cases when there is no ShedRead for each argument in the ins list of the instructions. However, it shouldn't be too difficult to add them in the instruction classes.
Nov 22 2019
Can you please clarify how the existing implementation hurts the scheduler and when fused pairs are then broken?
At first glance, this patch seems sensible, but I'm not sure that D70066 is necessary.
Nov 20 2019
Nov 12 2019
Nov 11 2019
Nov 4 2019
Oct 31 2019
Oct 30 2019
Oct 17 2019
🔔 ¡Ping! 🔔
Oct 11 2019
Oct 10 2019
Oct 9 2019
Oct 8 2019
Added the hex FP constants for easier auditing.
@efriedma, you had a point. I now trimmed the precision down to one digit short of seeing a change in the mantissa bits.
Oct 7 2019
Indeed, it seemed to be a coarse check in case the following transformations end up calling pow(). However, the only transformation that calls pow() is when shrinking to powf(), which itself chacks for the availability of this routine. So this patch seems to address this issue the wrong way. It seems that removing this check would be better.