- User Since
- Oct 27 2012, 6:35 AM (360 w, 1 d)
Scaling down ambigiousness of the patch: if not using BEXTR, only use BZHI if either the mask is larger than 32-bit, or the load folding will happen.
Sat, Sep 21
Please upload all patches with full context (-U99999)
Ah, finally got it. There is more general fold here:
Name: sub_ashr_and_nsw %sub = sub nsw i8 %X, %v %ashr = ashr i8 %sub, 7 %r = and i8 %ashr, %v => %cmp = icmp sle i8 %v, %X %r = select i1 %cmp, i8 0, i8 %v
Fixup comment, NFC.
Please change clamp0 everywhere to clamp negative to zero, it wasn't obvious to what clamp0 means until reading all of the patch.
This looks ok otherwise. Please wait for @spatel to comment.
Certainly missing a test.
Fri, Sep 20
I'd think this needs more test coverage.
We decided that load combining was unsuitable for IR because it could obscure other optimizations in IR. So we removed the LoadCombiner pass and deferred to the backend.
Thu, Sep 19
Why reimplement the wheel?
Does arc diff --update not work with git monorepo?
Does this still happen after rL372262?
Wed, Sep 18
Rebased by swapping patches around.
On Wed, Sep 18, 2019 at 9:02 PM James Molloy via Phabricator <email@example.com> wrote:
jmolloy accepted this revision.
This revision is now accepted and ready to land.
I think some patches just severely lack words.
It is entirely non-obvious to me why this fold should happen, yet the patch description is empty.
Hm, i thought some previous patch already adds llvm ir riscv-specific intrinsics for those.
Tue, Sep 17
Upload correct diff with no noise in tests..
With new-pm soon being the default, i'm curious what's the plan with -ftime-trace?
Will it simply turn into a pumpkin? Is there an upcoming patch (along the lines of D62067#1511128)?
Inline asm is really unfriendly to the optimizer.
Ideally the plan should be to incrementally getting rid of it as soon as backend learns to properly match particular builtin.
Mon, Sep 16
- Docs missing
- How does this play with C++17 aligned new? Assuming compiler/library support for that isn't broken ('bye, OSX!', ?), i'm not sure why it would be UB for C++17, see https://godbolt.org/z/kwxRbu vs https://godbolt.org/z/om-bR2
- I'm not sure what's getCharAlign() has to do with anything here? You want to check the alignment requirement against the __STDCPP_DEFAULT_NEW_ALIGNMENT__ (i don't recall how to get it inside of clang) i would think?