[llvm] [Analysis] atan2: isTriviallyVectorizable; add to massv and accelerate veclibs (PR #113637)

Tex Riddell via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 8 15:59:44 PST 2024


tex3d wrote:

@farzonl 
> See this pr: https://github.com/llvm/llvm-project/pull/95517/files
> tests like this should show that the libcall name to intrinsic mapping in the check line
> llvm/test/Transforms/SLPVectorizer/X86/call.ll

This still passes without the mapping for `tan`.  There are other places where libs calls are mapped, which is why I'm unsure of what actually depends on this code.

I commented out the whole block to see if any tests would fail, and found exactly one:
https://github.com/llvm/llvm-project/blob/cb98366ea4ce02e739eb4091c6227b67b60616c9/llvm/test/Transforms/TailCallElim/inf-recursion.ll#L3-L13
If `isLoweredToCall` returns true for `fabs`, this results in:
```ll
define double @fabs(double %f) {
entry:
 br label %tailrecurse
tailrecurse: ; preds = %tailrecurse, %entry
 br label %tailrecurse
}
```

This doesn't exactly seem like the primary effect this code is meant for.  From call graph inspection, it looks like this could impact some loop unrolling if it contains a matching call.  It doesn't look like any of these were explicitly tested though.

Architecturally, this is ugly, as is pointed out by the comment from 10 years ago in that code:
https://github.com/llvm/llvm-project/blob/cb98366ea4ce02e739eb4091c6227b67b60616c9/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h#L152-L155

It seems like we should just have tablegen definitions with clear properties somewhere, then all these random spots in code that depend on specific properties would be handled in a uniform way.

I guess I'll just merge it as-is for now.

https://github.com/llvm/llvm-project/pull/113637


More information about the llvm-commits mailing list