[PATCH] D149627: [NFC][SLP] Cleanup: Simplify traversal loop in SLPVectorizerPass::vectorizeHorReduction().

Vasileios Porpodas via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 3 10:55:43 PDT 2023

vporpo added inline comments.

Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:14097
+        assert(Stack.empty() &&
+               "This checks that changing `continue` to `break` was correct.");
+        break;
vporpo wrote:
> ABataev wrote:
> > vporpo wrote:
> > > ABataev wrote:
> > > > Replace the message with something meaningful, like 'Empty Stack is expected.'
> > > I think we should also mention why we are checking that Stack is empty, so that we know how to fix this if it triggers the assertion. How about: "Empty Stack expected. Change `break` to `continue`?"
> > The change continue to break is actual only for this patch, not for the codebase. So just `Empty stack expected, the original root is reached` or something like this should be enough
> I don't understand what you mean. We are adding the assertion specifically for checking that changing continue to break was correct, and you are arguing that we should not mention this in the assertion message, and instead write some generic message that provides no information about why this assertion exists. 
> How will a message like "Empty stack expected, the original root is reached" is supposed to help a third person fix the code if the assertion triggers?
Anyway I don't feel to strongly about this I will change it to what you are proposing.

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list