Mostly LLVM work.
- User Since
- Nov 2 2018, 7:48 AM (115 w, 6 d)
LGTM. The issues we detected are now solved.
Thu, Jan 7
- Sprayed test with correctness mist.
Mon, Jan 4
Sun, Jan 3
Sat, Jan 2
Based on your description and rationale I *think* this makes sense and should be safe. I'll leave it to others to explicitly approve the patch, though.
Mon, Dec 28
(Now, that C++ has lambdas, etc, I wonder if using a standard find algorithm would be even simpler/cleaner?)
Dec 9 2020
Dec 8 2020
Dec 7 2020
Add float Complex case, for regression test completeness.
Address review feedback.
Dec 3 2020
I'm concerned by the fact that this port doesn't clear the cache for other cores, which this builtin generally does. Furthermore, the #elif doesn't reliably detect baremetal, since it could be other non-Linux OSes, such as BSD. It's also not immediately obvious who actually requires / benefits from this builtin. Lastly, the existing test won't run for baremetal, so this won't get tested.
We've been pretty consistent with trying to align our behaviour with the GNU tools, so these kinds of patches have always been welcome. I'm not sure if there are compatibility concerns with changing the behaviour now. Should we have a an option for changing the printing style, both for compatibility and personal preference?
About the verification, I'm not sure what the best approach is.
Dec 1 2020
Nov 25 2020
Nov 24 2020
Merge tests, using --docnum.
Overall this still looks good to me. Could you please just fix the following inconsistency?
Nov 23 2020
I had added an inline comment for half-arith.ll nitpicking that it should probably use a hard-float ABI, to cut down on the fmvs. Now it has test checks for both soft float and hard float ABIs. @HsiangKai Is this because you think there is value in testing both scenarios in that file (and several others)?
Nov 20 2020
It seems a bit excessive to me to coalesce the entry points into bundles of 4. Do you have any particular benchmarking data or reasoning that supports choosing that threshold?
Also, shouldn't this implementation include CFI directives?
Nov 11 2020
Fix bad test.
Nov 10 2020
This patch is pretty big, so I haven't yet been able to delve as deeply as I wanted to, but so far I didn't notice any major issues.
Thanks to Craig for providing the initial round of feedback, and pointing out several issues.
Some nitpicking remarks:
- Several tests should probably use a hard-float ABI, to cut down on the fmvs.
- For files that handle F, D and half, it would be nice to try to be more consistent regarding the order in which the code and declarations for those appears. (When handling early returns, short-circuit evaluation, and so on, it might arguably still make sense to be inconsistent and order things based on expected frequency, as that might have a very slightly compile-time performance impact)
Nov 9 2020
Nov 8 2020
Nov 6 2020
Nov 5 2020
Fix borked test update.
LGTM. I guess the old libcall was producing the correct result anyway, since the in-register representation is the same?
Address review feedback.
Nov 4 2020
Overall LGTM. See inline comment before committing.
NFC minor clean-up tweak.
Nov 3 2020
Add logic to balance increased instruction count vs decreased code size.
Arrr matey, I'm the captain now!
Nov 2 2020
LGTM. Good nitpicking feedback by @frasercrmck! :-)
Nov 1 2020
- Use MCBasedABI
- Remove ArchSpec core bits, to be moved to D86292
Oct 29 2020
Oct 28 2020
Overall, LGTM. Two things, though: