[PATCH] D47491: Expand constrained FP operations

Andy Kaylor via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 11 16:26:10 PDT 2018


andrew.w.kaylor added inline comments.


================
Comment at: test/CodeGen/X86/vector-constrained-fp-intrinsics.ll:5
+; CHECK-LABEL: constrained_vector_pow
+; COMMON: pow
+define <2 x double> @constrained_vector_pow() {
----------------
cameron.mcinally wrote:
> andrew.w.kaylor wrote:
> > I think this check needs to do more than this. You should be verifying the complete expansion.
> > 
> > Also, can you add checks for some other instructions. Some of these won't require expansion, right?
> > I think this check needs to do more than this. You should be verifying the complete expansion.
> 
> Thanks. I've added a check for the upper POW.
> 
> > Also, can you add checks for some other instructions. Some of these won't require expansion, right?
> 
> POW is the only strict operation that I've found that currently needs expansion on X86. That said, I have a full set of tests that cover each strict operation. I could add them if you'd like, but it's a bit superfluous for this change. I do intend to add the full set once other patches are sent upstream. Thought?
I'd really like to see checks of the entire sequence to which this gets expanded. That seems like the only way to be certain it was expanded correctly. In some of the other tests I took a shortcut similar to what you did here because there was a 1-to-1 correspondence between the intrinsic and the instruction to which it was lowered. Even in that case it was probably not an ideal test.

Regarding instructions that aren't supposed to be expanded, what I would like to see here is a test that uses a vector form of the constrained intrinsic and a check that verifies that we generated the corresponding vector instruction.


Repository:
  rL LLVM

https://reviews.llvm.org/D47491





More information about the llvm-commits mailing list