- User Since
- Apr 14 2013, 11:48 AM (352 w, 5 d)
The constrained fcmp intrinsics don't allow the TRUE/FALSE predicates.
Thu, Jan 16
What are the semantics of vfnmadb with respect to when it rounds vs the negation?
A few comments (see inline) -- otherwise this looks good to me, thanks!
Wed, Jan 15
Why do we need to copy this algorithm here? Can't you simply add this case to TargetLowering::expandUINT_TO_FP instead?
Tue, Jan 14
It turns out this patch introduced a regression with chain handling in code mixing constrained intrinsics and memory operations.
Mon, Jan 13
Sun, Jan 12
Looks like the remaining test case changes are strictly due to scheduling.
Address review comments and rebase.
Fri, Jan 10
Updated patch now that D72224 has landed. At this point, there are no test suite failures remaining.
OK, this looks all good to me now. I agree that there can be future API cleanup, but that can be done in follow-on patches. I think we should go with this for now.
Thu, Jan 9
Wed, Jan 8
In general this all makes sense to me. See a few minor inline comments.
Use SmallVector::append instead of ::insert.
Tue, Jan 7
Mon, Jan 6
Just a couple of general comments for now, not yet going into implementation details.
Fri, Jan 3
Thu, Jan 2
Mon, Dec 30
Check all matched nodes whether they may raise FP exceptions.
Wed, Dec 25
Updated to current mainline.
Otherwise, this LGTM.
Tue, Dec 24
Otherwise this LGTM. Thanks for taking care of those!
This also needs updating the test cases that are testing for the old behavior:
I've had a closer look at the vec-strict-fptoint-256.ll changes I mention here:
In one case this led to codegen changes (different register allocation) since some copy propagation is no longer run on instructions marked as fpexcept.
Mon, Dec 23
Another thing I forgot to mention: I've had to add a bit of code to the X86 target to set NoFPExcept flags. This is because you're using the same opcodes for both strict and non-strict compares, and adding the NoFPExcept in the non-strict avoids some codegen regressions we'd otherwise see.
Note that this patch has D71840 as prerequisite.
Sun, Dec 22
Fri, Dec 20
Can we simply move all the validity checks (arg only valid on SystemZ, or arg only valid in combination with another arg) into ParseCodeGenArgs in CompilerInvocation.cpp, where the args are initially looked at?
Dec 18 2019
LGTM with minor tweak. Thanks!
Minor cosmetic comment, otherwise this LGTM.
Dec 17 2019
Should the changes to llvm.minimum / llvm.maximum be applied now?
This is done using just raw text like gcc does it (see gcc@06477d3). Is this acceptable?
The new test case causes build bot failures (hidden by another failure that was already present):
It looks like this caused build bot failures:
It seems that the "backchain" attribute does not have to be checked this way, since it is simply present or non-present. Would this be preferrable perhaps also for "packed-stack" (instead of this patch)?
Dec 16 2019
OK, then I think this patch LGTM.
Added float (f32) test cases.
The diag::err_opt_not_valid_on_target message is emitted from CodeGenFunction::StartFunction -- does that mean if you use -mpacked-stack on a non-SystemZ target when compiling a file with many functions, you get this error message many times? If so, this should probably be moved somewhere else. (It looks like the existing check for -mnop-mcount may already have the same problem.