[PATCH] D97393: [OpenMP IRBuilder, MLIR] Add support for OpenMP do schedule dynamic

[Mats Petersson via Phabricator via llvm-commits]

Leporacanthicus added a comment.

In D97393#2666954 <https://reviews.llvm.org/D97393#2666954>, @Meinersbur wrote:

> Could you add the following to the unittest?
>
>   Builder.restoreIP(EndIP);
>   Builder.CreateRetVoid();
>   OMPBuilder.finalize();
>   EXPECT_FALSE(verifyModule(*M, &errs()));
>
> I checks whether the IR is internally consistent.

Good spot. Done!



================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:1741-1744
+  AllocaInst *PLastIter = dyn_cast<AllocaInst>(&*(AllocaIter++));
+  AllocaInst *PLowerBound = dyn_cast<AllocaInst>(&*(AllocaIter++));
+  AllocaInst *PUpperBound = dyn_cast<AllocaInst>(&*(AllocaIter++));
+  AllocaInst *PStride = dyn_cast<AllocaInst>(&*(AllocaIter++));
----------------
Meinersbur wrote:
> Leporacanthicus wrote:
> > Meinersbur wrote:
> > > [nit] Can use `auto *` when using `dyn_cast` on the same line. At least, please use consistent style in the same function,
> > The whole file is inconsistent in this respect. First dyn_cast in the file is to a named type (line 156), the second one is to an auto (line 161), but then it is MOSTLY named types, with a scattering of auto in the original static code.  I'm changing the ones below to use non-auto, to match the rest of the file. 
> Reviews of previous patches were not necessirily perfect ;-(
Indeed. Problem comes when you copy-paste code from previously "not perfect reviews". I've been on the other end of this too. ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97393