- User Since
- Sep 12 2012, 3:50 AM (384 w, 6 d)
Thu, Jan 23
Thu, Jan 16
Nice cleanup, thanks!
Dec 23 2019
Dec 10 2019
In general anything moving from 32-bit to 64-bit bitmasks in the backend LGTM. I'd even go further and suggest using std::bitset to remove this problem Once And For All, but that would require significantly more benchmarking.
In fact, one concrete improvement we could do with this CL (which clearly found everything we needed to patch) is introduce a typedef for "A bitmask of functional units". This would allow us to find all the instances again in future, and highlight the point of the bitwidth to users.
Dec 6 2019
Nov 22 2019
LGTM, sorry for the delay.
Nov 20 2019
Nov 14 2019
Nov 12 2019
LGTM, but in which cases are we enforcing const-correctness? const-correctness is not a property LLVM enforces almost anywhere because of its virality.
Nov 6 2019
Nov 5 2019
Oct 18 2019
Updated based on David's comments.
Oct 17 2019
Thanks Thomas! Done.
Thanks Graham for taking my feedback on board. I agree with Simon's request too.
Oct 15 2019
Oct 9 2019
Oct 7 2019
Oct 5 2019
Oct 4 2019
Fine from my perspective. The original changes were due to some language-lawyering to fix an awkward testcase, and I can completely believe my lawyering was wrong :)
Oct 3 2019
Oct 2 2019
Oct 1 2019
Address review comments.
Sep 30 2019
Looks trivially good to me.
Thanks David - I missed your comments, sorry this is a late response.
Sep 26 2019
This feels like it could cause a pretty serious regression. This essentially disables global merging with -fdata-sections, which I know at least one linker relies upon for code size.
Sep 25 2019
Sep 24 2019
LGTM, sorry for the long latency.
Sep 21 2019
Sep 20 2019
Thanks for pushing the revert. Apologies!
Thanks for pushing the rollback Mitch!
Thanks Jinsong! I've committed this as rL372376.
Sep 19 2019
Sep 18 2019
Ah great, thanks for the context, and I'm glad this patch helps you out!
Thanks Simon! The patch was uploaded with -U999, but this one's with -U9999 just in case :)
Thankyou Daniel! Looks great!
Sep 17 2019
Sep 16 2019
I will be very surprised if this does not cause any regressions at all.
I can think of a rationale for the current behaviour. A lopsided if-triangle could indicate it might be better to emit a branch than speculate. Especially as this transform is context-free, input code that contains the same pattern many times can cause an explosion in code size and the transform was likely designed to be conservative.
Sep 15 2019
Adding Simon and Krzysztof as the original authors of InfoByHwMode.
Sep 13 2019
Sep 12 2019
Thanks for the great feedback Jinsong! I've addressed your comments.
Sep 9 2019
Sep 6 2019
Sep 5 2019
Sep 4 2019
No-asserts build fixed in rL370894. Apologies for the breakage, I was holding git wrong.
On the other hand, I don't really want to spend much time thinking about how to make SelectionDAG better at this point.
Sep 3 2019
Your problem sounds more like we need a better system for checking if a store/trunc store is legal
(for more context, this is in the context of https://reviews.llvm.org/rL370576 . It's entirely possible I picked a suboptimal target hook, but allowsMemoryAccess seemed correct to me.).
Aren't the targets intended to override allowsMisalignedMemoryAccesses? Does that not work for your usage?
Thanks for the review Thomas!
Landed in rL370705. Thanks Thomas!