[PATCH] D114940: [OMPIRBuilder] Support ordered clause specified without parameter

Michael Kruse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 28 14:26:05 PST 2022


Meinersbur added inline comments.


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2025-2028
+  AllocaInst *PLastIter = dyn_cast<AllocaInst>(&*(AllocaIter++));
+  AllocaInst *PLowerBound = dyn_cast<AllocaInst>(&*(AllocaIter++));
+  AllocaInst *PUpperBound = dyn_cast<AllocaInst>(&*(AllocaIter++));
+  AllocaInst *PStride = dyn_cast<AllocaInst>(&*(AllocaIter++));
----------------
I think assuming a specific order of allocas and other instructions is not that useful. They break easily with any change and are hard to repair since the instruction order has no meaning. The code here effectively only tests "there are 4 alloca instructions at the beginning of the function"

I would prefer we only have few, but significant tests such as "there is a call to __kmpc_dispatch_init_4u somewhere". Even searching for an instruction with a specific name is more meaningful than relying on the order.


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

https://reviews.llvm.org/D114940



More information about the llvm-commits mailing list