- User Since
- Feb 20 2018, 9:31 AM (227 w, 2 d)
Tue, Jun 28
Move more of Instruction::isSafeToRemove() into this function. The remainder of isSafeToRemove() isn't needed because we filter out function calls at the top of the changed function.
Mon, Jun 20
Thu, Jun 16
Update for review comments. I've also reduced it down to just fcmp and fcmps having a functional impact. I don't have a good way to test the binary operators just yet.
Mon, Jun 6
Fri, Jun 3
Jun 1 2022
Update comment as requested.
On targets that support static rounding mode (like RISCV) dynamic and constant rounding modes may be different and the behavior changes if default mode is replaced by dynamic.
Whether a target supports static rounding modes on floating-point instructions is completely irrelevant. The user-visible behavior must be the same either way. If a target doesn't have specialized instructions, the code generator can save/restore the rounding mode. This should be transparent to the user; the user can't read the rounding mode in between the save and restore. (We already do this sort of rounding mode manipulation on x86, to implement float-to-int conversion on targets without SSE.)
May 31 2022
Update for review comments: eliminate unneeded checks.
May 25 2022
May 24 2022
I've verified that it fixes the build on FreeBSD 11.1 and the current version of FreeBSD 12.
May 23 2022
Will this fix FreeBSD 12 (and maybe 11?). FreeBSD 12 is still very much supported.
May 13 2022
May 3 2022
May 2 2022
Apr 26 2022
Update for review comments.
Apr 21 2022
Apr 19 2022
Update for review comments.
I realized that going through isInstructionTriviallyDead() means it will handle constrained intrinsics differently than it does now. So this isn't strictly speaking an NFC change.
Not needed anymore after D69798 landed.
Apr 14 2022
Apr 12 2022
I trust you on the instruction set changes. LGTM.
Apr 8 2022
Apr 5 2022
Mar 25 2022
Mar 21 2022
Mar 18 2022
Mar 17 2022
It's been a while, but I think the aarch64-neon-intrinsics-constrained.c test is trimmed down from the aarch64-neon-intrinsics.c test. Shouldn't the constrained and non-constrained end-to-end tests be treated the same?
Mar 11 2022
Feb 22 2022
I'm still not clear on why, when a transform pass knows an instruction can be deleted, we are required to keep that instruction around anyway. It seems like these dangling, useless instructions would interfere with subsequent optimizations until finally they might or might not get removed here. And constant folding isn't sufficient to duplicate the logic of the transform pass that determined the instruction could be deleted.
Feb 16 2022
Please wait a couple of days in case there are any comments from anyone else.
Feb 14 2022
Remove unneeded includes, and correct the placement of the include of FMF.h.
Feb 10 2022
Update for review comments: don't fold away a case that can and should result in -0.0. Add tests for this very case.
Feb 3 2022
Feb 1 2022
No, actually wouldInstructionBeTriviallyDead() rejects some instructions that the previous isSafeToRemove() function allowed. By trusting a true returned by wouldInstructionBeTriviallyDead(), but using the old logic if it returned false, we can make this change NFC. To that end I have also removed the constrained FP logic and I plan on adding it back in probably D115737.
Jan 28 2022
Jan 27 2022
Jan 5 2022
In IRBuilder.h we have this:
Dec 21 2021
Dec 15 2021
Dec 14 2021
Dec 6 2021
I don't see how this will resolve my issue. I'll start a new thread on llvm-dev. Don't let me block this review.
BTW, for a future ticket: having curl enabled without first checking to see if it is installed creates builds failures on non-Linux hosts. OK, yes, it can be disabled with a cmake argument, but it would be better if the extra cmake argument wasn't needed. A warning at cmake configure time that says what features will be missing without curl is probably a good idea as well.
Dec 2 2021
That flag tells front ends that strictfp will generate correct instructions. Don't set it if more work is needed for correct instructions. The support should at least have parity with X86 and SystemZ.
Dec 1 2021
Nov 30 2021
Well, the problem is more widespread than that. IPSCCP can create instructions where it knows the result, and it knows the instruction will never trap. Consider how it can propagate two constants into a compare instruction. It knows the values are constants, it knows they are not undef, poison, NaN, Inf, it knows the value of the compare, and it uses that knowledge to further propagate constants and simplify the code. But it can't remove the constrained compare instruction because we don't have a good way to tell it the instruction has no side effects. It does call Instruction::isSafeToRemove(), and it does _not_ call wouldInstructionBeTriviallyDead(). A more centralized approach would be helpful.
Oct 25 2021
Oct 21 2021
Replaced with D112256. This patch here has restrictions that are not needed. To avoid confusing reading I'm replacing the review.
Oct 15 2021
Everything that can be tested is now pushed in other tickets. I'll keep this around as a reminder to come back when m_FSub() is updated, but it doesn't need any action from anyone right now.
OK, this is overkill per Andy's comment. I'll update.
Oct 14 2021
Oct 13 2021
As requested, add tests for rounding to negative infinity, exceptions ignored, no sized zeros fast math flag.
Oct 12 2021
Update for review comments.
Oct 11 2021
Oct 8 2021
Oct 7 2021
Oct 6 2021
Oct 4 2021
Sep 30 2021
As for llvm langref, I think the following statement is cited here: https://llvm.org/docs/LangRef.html#floating-point-environment:The default LLVM floating-point environment assumes that floating-point instructions do not have side effects. Results assume the round-to-nearest rounding mode. No floating-point exception state is maintained in this environment. Therefore, there is no attempt to create or preserve invalid operation (SNaN) or division-by-zero exceptions.
This sentence is about exceptions but not the results of operations. So actually nothing in the langref justifies this transformation.
The wording could be made more explicit, but as I suggested in my comment from Aug 6 in this review: the default LLVM FP env ignores exceptions, so citing the IEEE-754 clause that begins with "under default exception handling" to describe SNAN->QNAN behavior is not applicable.
This is peculiarities of IEEE-754 language. Exception handling in this context is what to do if exceptional situation occurs. In 7.1 there is a description of the default exception handling:This clause also specifies default non-stop exception handling for exception signals, which is to deliver a default result, continue execution, and raise the corresponding status flag (except in the case of exact underflow, see 7.5).
This is exactly the way the exceptions are handled in the default FP environment. Exception must be handled in some way because hardware always signals them if exceptional situation occurs.