[LoopVectorizer] give more advice in remark about failure to vectorize call

[spatel]

Summary

Something like this is requested by:
https://bugs.llvm.org/show_bug.cgi?id=40265
...and it seems like a common enough case that we should acknowledge it. Not sure if this crosses the line for wordiness in an optimization remark though.

[efriedma]

Can we check whether the function could be vectorized if fast math were enabled, so we only show the advice when it's relevant?

"relaxing the floating-point model" is a little confusing... can we explicitly say "consider turning on fast math" or something like that?

[spatel]

Patch updated:

1. Try to distinguish a vectorizable libcall from an arbitrary call (I don't see an exact mapping, but "hasOptimizedCodeGen()" looks close).
2. Add tests to show that we correctly differentiate the 2 cases.

[hfinkel]

lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

727

I'd prefer it say "fast-math mode" instead of just "fast-math". It would be nice if we could also point users to -fno-math-errno, as that might fix this problem for them and they might not be able to use -ffast-math for the whole translation unit.

Now we already have a problem in the vectorizer because it has a lot of optimization remarks that mention Clang-specific things (flags, pragmas, etc.). The intent of the optimization-remark design was that the frontend callback handler would handle such cases by adding frontend-specific information in the frontend (and not have it embedded here). That didn't happen, and while we should clean this up, in the mean time we might just make the problem incrementally worse and mention flags here too: "try compiling with -fno-math-errno or -ffast-math".

[spatel]

lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

727

Yes, I was trying to avoid clang-specific language here.
And in the motivating bug report, you're exactly right -- we only needed -fno-math-errno to overcome the limitation (that's why I was using the likely too vague "relaxed FP" vocabulary in the previous rev).

[spatel]

Patch updated:

1. Added an FP-type constraint to the mathlib check (no point suggesting FP flags if it's not an FP call).
2. Changed remark text to include clang-specific flags (and suggest/hope that users can translate those to their actual front-end options if this isn't a clang-based invocation).

[hfinkel]

LGTM

[Ayal]

This LGTM too, just adding mtcw wondering if these extra checks for more accurate reporting are worth placing under allowExtraAnalysis(); and/or if TLI->isFunctionVectorizable() shouldn't be the one informing the cause of its failure when returning false.

[spatel]

In D56551#1354518, @Ayal wrote:

This LGTM too, just adding mtcw wondering if these extra checks for more accurate reporting are worth placing under allowExtraAnalysis(); and/or if TLI->isFunctionVectorizable() shouldn't be the one informing the cause of its failure when returning false.

Those are good questions/comments. I'm not too familiar with the code organization here, but I'll add that to the 'TODO' comment for now, so we don't lose it.

lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

for (Instruction &I : *BB) {
       //   * Have a mapping to an IR intrinsic.
       //   * Have a vector version available.
       auto *CI = dyn_cast<CallInst>(&I);
       if (CI && !getVectorIntrinsicIDForCall(CI, TLI) &&
           !isa<DbgInfoIntrinsic>(CI) &&
           !(CI->getCalledFunction() && TLI &&
             TLI->isFunctionVectorizable(CI->getCalledFunction()->getName()))) {
         ORE->emit(createMissedAnalysis("CantVectorizeCall", CI)
-                  << "call instruction cannot be vectorized");
+                  << "call instruction cannot be vectorized "
+                     "(if this is a math library call, consider relaxing the "
+                     "floating-point model)");
         LLVM_DEBUG(
             dbgs() << "LV: Found a non-intrinsic, non-libfunc callsite.\n");
         return false;
       }
 
       // Intrinsics such as powi,cttz and ctlz are legal to vectorize if the
       // second argument is the same (i.e. loop invariant)
       if (CI && hasVectorInstrinsicScalarOpd(
                     getVectorIntrinsicIDForCall(CI, TLI), 1)) {
         auto *SE = PSE.getSE();
         if (!SE->isLoopInvariant(PSE.getSCEV(CI->getOperand(1)), TheLoop)) {
           ORE->emit(createMissedAnalysis("CantVectorizeIntrinsic", CI)
                     << "intrinsic instruction cannot be vectorized");
           LLVM_DEBUG(dbgs()
                      << "LV: Found unvectorizable intrinsic " << *CI << "\n");