[PATCH] D106843: [IVDescriptors] Fix bug in checkOrderedReduction

Tue Jul 27 08:03:26 PDT 2021

anna added inline comments.

================
Comment at: llvm/test/Transforms/LoopVectorize/fp-reduction-crash.ll:1
+; REQUIRES: asserts
+; RUN: opt < %s -loop-vectorize -S | FileCheck %s
----------------
fhahn wrote:
> fhahn wrote:
> > anna wrote:
> > > kmclaughlin wrote:
> > > > nit: I don't think `REQUIRES: asserts` is necessary for this test?
> > > This is to show that the failure only occurs in assertions enabled mode. It will not fail in any other mode.
> > > 
> > > To put it differently, if I first landed this testcase alone, it will succeed in non-assertion enabled mode and it isn't clear what the problem is (since we cannot add `XFAIL`). 
> > > 
> > Can you add check lines to make sure the IR is transformed as expected? That way, we do not need to rely on the asserting causing the test to crash.
> actually, can the test be added to the file already containing order reduction tests, with a comment explaining what it checks? That will help make maintaining it easier.
The reason it doesn't cause any failure in non-assertion mode is that this just checks and bails out without doing vectorization. The bail out was too late. That was the only reason this crash occurred. 
So, the IR is not transformed for this reduction pattern - not sure what the CHECK-LINES will do. I will add a comment explaining what it checks and move it to the order-reduction tests.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106843