[PATCH] D96462: [LV] Add remarks that explicitly mention error handling in candidate loops

[Joseph Huber via Phabricator via llvm-commits]

jhuber6 added a comment.

In D96462#2556207 <https://reviews.llvm.org/D96462#2556207>, @fhahn wrote:

> Thanks for working on improving the remarks! Unfortunately we have to be careful to keep them generic with respect to the frontend. I don't think we should be  using a function name string to detect using `assert` and include other implementation specific parts (like `NDEBUG`) in the message. For example, the user could define their own `__assert_fail` function or the IR could be coming from a Rust or Swift frontend.

The motivation to handle `__assert_fail` specifically as a secondary case is mainly because it has a simple solution we can put in the message. It might be sufficient to only check for `unreachable` instructions because the diagnostics message should point to the offending assertion and leave it to the user to know how to turn them off.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:1078
+            "loop control flow is not understood by vectorizer. "
+            "Loop contains control flow that does not return",
+            "CFGNotUnderstood", ORE, TheLoop, U);
----------------
fhahn wrote:
> In this case, the loop *technically* does not contain the `unreachable`, right? One of the exit blocks does. I am not sure how to best phrase it, but it might be worth adjusting the message below `reportVectorizationFailure("The loop must have a unique exit block",` to include the information that some of the exit blocks end in `unreachable`
> 
> Unfortunately I don't think the user can do much with this information.
Yes, the loop shouldn't contain an `unreachable` instruction, but the error handling code will be inside the loop and will create a new exit block to handle the error. This is just a specialization of failing because of multiple exit blocks. The main benefit of checking it separately here is that the diagnostics will point to the expression that caused it. Some of these situations have simple solutions, such as recompiling without assertions or doing manual loop unswitching.

Also. should the remarks be prefaced with "loop control flow is not understood by vectorizer." In this case it will print another remark saying the same anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96462