[flang][OpenMP] Set OpenMP attributes in MLIR module in bbc before lo…

[kparzysz]

…wering

Right now attributes like OpenMP version or target attributes for offload are set after lowering in bbc. The flang frontend sets them before lowering, making them available in the lowering process.

This change sets them before lowering in bbc as well.

[agozillon]

LGTM, nice catch! Seems that as the attribute set expanded we forgot to move this... it's a bit of a pain to keep both bbc and flang in synch at times unfortunately

[clementval]

It looks like this triggers some failures in the pre-commit CI.

[agozillon]

LGTM, nice catch! Seems that as the attribute set expanded we forgot to move this... it's a bit of a pain to keep both bbc and flang in synch at times unfortunately

It does however appear to break a few dozen buildbot tests, but that may be something unrelated in the main branch you have this PR rebased on. Or perhaps the burnside.getModule() line also needs to be moved to below the lower call, at least I hope it's something as simple as that and not the tests acting a lot differently after the attribute movement.

[kparzysz]

It worked with my version of main, I guess it wasn't recent enough. Looking into it...

[kparzysz]

This still worked for me with the current main. Interestingly enough, it did pass on Windows in CI. Merging main into this PR to see if that fixes the issue.

[agozillon]

This still worked for me with the current main. Interestingly enough, it did pass on Windows in CI. Merging main into this PR to see if that fixes the issue.

Lovely, the inverse of what normally happens (the windows buildbot falling over)! Perhaps the merge and a second run will fix the issue. If it doesn't I can perhaps do a rebase/rerun on one of my open PRs and see if it triggers the same problem, to see if it's just something a little strange going on with the buildbot environment if that's any help.

[kparzysz]

It's a dangling reference problem: #82800.

[kparzysz]

I normally use gcc and it worked. I rebuilt everything with clang and it started failing.

Edit: I use gcc when building flang at work, not "normally", heh.

[agozillon]

That's a weird and I imagine painful one to find and fix.

[agozillon]

Thank you for finding the insidious little error! I've accepted the other PR as it seems reasonable to me (and passes all tests cleanly on linux), I am not code owner of the tool though, so if you feel a little unsure about the change then feel free to wait on someone else to chime in (although, from what I can see it's a fix that won't affect anything functionally other than making the code correct). Afterwards if this set of buildbot tests passes on rebase my acceptance still stands, so I think it should be safe to land it.

[clementval]

LGTM. Maybe just rerun the precommit CI once your other change has landed to be sure but it should be fine.

[kparzysz]

Windows CI failed for an unrelated reason.

[agozillon]

Windows CI failed for an unrelated reason.

Definitely an annoying and very frequent issue at this point.

Thank you for the fix!