[PATCH] D64916: [LV] isScalarEpilogueAllowed. NFC.

[Hideki Saito via Phabricator via llvm-commits]

hsaito added a comment.

Looks to be a good change. Can we add a little more improvement to this patch --- adding more crispness in ORE message?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4683
         dbgs()
-        << "LV: Aborting. Runtime ptr check is required with -Os/-Oz.\n");
+        << "LV: Aborting. Runtime ptr check is required.\n");
     return None;
----------------
I understand that the existing code didn't properly reflect having -Os/-Oz but losing this, where applicable, isn't great. This being a debug message, I won't insist, since "performing code size checks" debug message is already printed.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4752
                "Enable vectorization of this loop with '#pragma clang loop "
                "vectorize(enable)' when compiling with -Os/-Oz");
   return None;
----------------
Ideally, this is where we'd like to be crisp about whether the code was actually compiled under Os/Oz or vectorizer itself is forcing no scalar epilogue. If the flag is used, programmer has an option of using pragma or getting rid of the flag. If vectorizer is forcing, the only choice is using pragma.

In order for us to be able to do that, IsScalarEpilogueAllowed, when it is false, should come with reasoning behind it. May want to consider changing it to have enum value instead, e.g., ScalarEpilogueAllowed, ScalarEpilogueNotAllowedOptSize, ScalarEpilogueNotAllowedLowTripLoop.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64916/new/

https://reviews.llvm.org/D64916