This adds match patterns for the reassociated forms of FNMUL.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Can we please more verbose title for the patch?
llvm/test/CodeGen/AArch64/fnmul.ll | ||
---|---|---|
54 | The test cases cover -x * y test cases. Can we have x * -y too? I believe this patch will handle those cases too. However, it is good to have a test case. |
Added the tests.
I think this subject + body is an excellent description of the things here:
[AArch64] FNMUL This adds match patterns for the reassociated forms of FNMUL.
I could merge this all into the subject, have no body, but I don't think this is all very important for this simple patch...
NoHonorSignDependentRounding is the old way of doing things - it should be based on fastmath flags nowadays. Or at least I've not heard of NoHonorSignDependentRounding before and it seems to be only used in the Arm backend at the moment.
This alive test seems to suggest that we can just always do the transform: https://alive2.llvm.org/ce/z/5-LdY1. Which I think sounds OK considering that it is a fneg. Do you know why HonorSignDependentRounding is needed? Was it just copied from Arm?
Thanks Dave.
Yeah, I also think we don't need NoHonorSignDependentRounding, but just added it to be consistent with ARM. The default is off, so it will do the transform, which is why I didn't mind adding it... but you're right it's probably better not to add it if it is the old way of doing things. So I will remove it from here. Let's then also remove it for ARM, but we can do that separately.
- removed NoHonorSignDependentRounding from the rules
- changed tests to run with and without fp-contract=fast, and some cases with and without fast IR instruction to show it should always trigger.
The test cases cover -x * y test cases. Can we have x * -y too? I believe this patch will handle those cases too. However, it is good to have a test case.