[Mlir-commits] [mlir] [MLIR][OpenMP] Prevent loop wrapper translation crashes (PR #115475)

[Kiran Chandramohan]

kiranchandramohan wrote:

> Thanks @kiranchandramohan for the quick review!
> 
> > Can we write a test for this?
> 
> At the moment we can't because this code path is not triggered by anything. Translation functions for loop wrappers currently call this function passing the region of the nested `omp.loop_nest`, so a loop wrapper region is never passed here. However, if you check the code directly below you can tell that this is doing the same thing as if an `omp.terminator` was present.
> 
> I think it's preferable to have this implemented before needing it rather than having it crash later on, since I think it was an oversight from when I implemented #112229. But let me know if you don't agree and I can keep it around until some composite construct lowering ends up triggering the issue.

The danger with this is that someone can remove this piece of code in refactoring and think it was accidentally introduced since no test fails. Ideally, such code can be introduced in the first patch that needs it. You can organize such patches as two commits, where the first commit is this patch and the second commit is the one that needs it

Note: I did not intend to request changes. I was trying reviewable.io and did not realise that it requested changes. 

https://github.com/llvm/llvm-project/pull/115475