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

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

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.



================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:1736-1737
+  // The returned value should be the "after" point.
+  ASSERT_EQ(EndIP.getBlock(), CLI->getAfterIP().getBlock());
+  ASSERT_EQ(EndIP.getPoint(), CLI->getAfterIP().getPoint());
+
----------------
Do not use `CLI` after it has been invalidated.


================
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++));
----------------
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 ;-(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97393