- User Since
- Feb 20 2018, 9:31 AM (275 w, 1 d)
Tue, May 23
Would a constrained version of this be needed?
Wed, May 17
There's a lot of code here. I'll try to look at it in the next couple of weeks. I was out last week and I'm a little behind.
Why delete from class ENDRecord the support for having the entry point specified on the END card? Wouldn't that be useful in a dumping tool at least? Or is this handled somewhere else now?
Wed, May 3
Tue, May 2
May 1 2023
The constrained intrinsic version is not documented here.
Apr 26 2023
LGTM as well.
Apr 21 2023
Are we certain that a constrained sqrt() folded away will be as accurate as not folded when the rounding mode is different? There's a lot of permutations of library calls and instructions that constrained sqrt() can be lowered to, and I'm worried that the results will be surprisingly bad with different rounding modes. If we don't fold then the end user can deal with library issues themselves at least, and hopefully square root instructions will be on CPUs that properly handle the different rounding modes.
Apr 20 2023
Apr 18 2023
Seems reasonable. LGTM.
Apr 14 2023
LGTM "for reals" this time.
Apr 13 2023
Apr 12 2023
Once the prereqs are in place this LGTM.
Apr 7 2023
Verify that CallBase::isStrictFP() continues to do what we think it is doing.
Apr 5 2023
Update for review comments.
Mar 29 2023
I don't see any tests for a constrained/STRICT tan. We do have tests for constrained sin and cos so tan tests where sin and cos are tested is probably appropriate.
Mar 24 2023
Mar 21 2023
Mar 15 2023
Where is fpclassTestIsFCmp0 implemented?
Mar 6 2023
This is no longer needed as the pieces have all made it into the tree already.
Mar 3 2023
Feb 23 2023
Feb 21 2023
Feb 16 2023
What's the plan for tying this to strictfp? Because I don't it should be tied to cases where we use the constrained intrinsics but the exceptions are ignored and the default rounding is in stated. Those instructions are supposed to behave the same as the non-constrained instructions. So keying off the presence of the strictfp attribute on the function definition, or the (equivalent) presence of constrained intrinsics, would be too simple.
Feb 9 2023
Feb 8 2023
I think that about covers it. LGTM.
Feb 6 2023
Feb 1 2023
We use "dynamic" for the constrained intrinsics. I'd stay consistent with our terminology and stick with "dynamic" here.
I like the new wording. Taking the list of exceptions out was also a good idea.
Jan 26 2023
Jan 20 2023
Jan 19 2023
Here it is: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/CloneFunction.cpp#L390
Jan 18 2023
Jan 17 2023
Are we testing _Complex multiply or subtraction anywhere? I have a vague memory of multiply not working correctly.
Dec 19 2022
Dec 13 2022
Dec 8 2022
Assuming we don't have to worry about the new, 8 and 16 bit FP formats I'm happy to see this move forward.
It looks like clang does have at least one test that checks for the strictfp attribute on function definitions. We also have a number that test for the strictfp attribute on function calls. So I think our test coverage is in good shape.
Are we certain that a value that is very close to infinity won't trip over into infinity after rounding?
Dec 5 2022
I believe this is correct. I'm looking at n3054 Annex F and it says that sqrt(+inf) returns +inf. But since you are checking that the input cannot be inf it follows that the output cannot be inf.
Oct 28 2022
Oct 24 2022
Abandoned in favor of D136466.
Update for review comments: allow folding for more than constrained intrinsics.
Oct 21 2022
Oct 18 2022
Oct 17 2022
It looks like by 754 sec 7.3 a (-x)/(+0) == -Inf, but the nsz is documented as saying the sign of the result is "insignificant", so I think we're good here.
Looks like IEEE 754-2019 section 7.3 says this should result in Infinity, but the ninf flag excludes Infinity, so poison seems correct.
Oct 3 2022
Update to use more targeted tests.
Sep 23 2022
D118387 was my ticket, and it was handled that way to preserve the existing behavior. I don't believe a wrapper is needed in this case.
Update for review comment: just use wouldInstructionBeTriviallyDead().
Aug 16 2022
Aug 12 2022
I was thinking that adding a test for strict exception behavior might make it look intentional that this true/false propagation isn't happening. But a test case with a note is perfectly fine.
Aug 11 2022
Aug 10 2022
Aug 5 2022
Address review comments.
Aug 1 2022
I'll get back to you on the tfpropagation.ll question. The test precommit:
I'll update the patch later today with that missing assertion.
Jul 28 2022
Update diff after precommit of tests.
Jul 25 2022
You know, I now believe that I can simplify this down to just this. The complication to treat constrained intrinsics as a memory update was put in when I added the restrictions on doing CSE across function calls. But since that restriction isn't needed I don't see the need for the rest of the complications. I don't think we need to go to heroics just for exception handlers, at least not at this time. This code is now much simpler.
Jul 5 2022
Jun 28 2022
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.
Jun 20 2022
Jun 16 2022
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.