[flang-commits] [flang] [Flang][OpenMP] Add --cg-rewrite pass to convert-to-llvm-openmp-and-fir.fir test (PR #96530)

[via flang-commits]

agozillon wrote:

> > > The fir::cg::XDeclareOp is quite a recent addition so it was not there when the test was originally written.
> > 
> > 
> > Thank you very much @abidh ! Would adding the cg-rewrite pass to the fir-opt argument list be the correct fix in this case then?
> > The verification issue was there prior to the addition of fir::cg::XDeclareOp, however, It seems from looking at the previous code (and quickly testing it via regressing that section of code) for the DeclareOp rewrite inside of PreCGRewrtie.cpp it would be correct still, as it seems to replace all DeclareOp's with the corresponding memory reference (if I am understanding the prior rewrite code correctly at least) which allows the FIR -> LLVM dialect conversion to pass without verification issues.
> 
> I don't think it will be problem adding `--cg-rewrite` here as it is already running `--cfg-conversion`. Although I wonder if we are testing the conversion to llvm-ir, why not take the IR right before that pass and check that specific conversion. But I don't have the full context so feel free to ignore my comments.

We could do that if we wish, I don't mind what direction we take, all it would require is an adjustment of the tests I added in another PR, but it does mean any IR that you generate via emit-fir can't be directly utilised in the test and you have to go out of your way to appropriately retrieve the IR directly before the pass. It's also came up a few times in our internal chat why certain FIR operations trigger verifier error in certain cases, so generally nice to find out why in this case!

If @kiranchandramohan or @tblah or anyone else has any further input to push this PR in one direction or the other (e.g. closure or landing) it'd be appreciated.  

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