Apologies for not accepting this earlier.

Addressed SingleBlockImplicitTerminator comment.

Thank you for the reviews @kiranchandramohan and @peixin. I will address the terminator comment soon. I have been busy with some other work recently. Apologies for the delay.

Rebase with main.

LGTM.

Thanks for addressing the comments @peixin. One minor comment, rest LGTM. (Please wait for one more review because I am not an expert in this part of the code).

Thanks for the review @kiranchandramohan and @peixin. Addressed comments and rebase with main.

LGTM.

In D116396#3215689, @peixin wrote:

@shraiysh Can you go ahead and try this method (using atomic.read in atomic.capture structured-block) and verify it in D115851? It may be better to show one whole design instead of part of it.

Another question is that will atomic.write and atomic.update will be used in atomic.capture structured-block and should their syntax also be updated?

Thanks Kiran. The i64 vec change LGTM (I still didn't understand why the alignment has to be 8).

Thanks for the review @kiranchandramohan. Addressed comments.

Fix missing semi-colon error

Depends on D116396

Updated patch to use omp.atomic.read and omp.atomic.update

Thanks for this patch @peixin. Can you please provide a reference to this requirement? (I was looking for the __kmpc_doacross_post in the RTL reference here but could not find it)

Changed argument variable names to be more consistent with other operations and the standard.

Thanks @peixin. Can you please review this patch?

@peixin I think we can merge this to make progress and if Kiran has any post-commit comments, we can address them later.

Thanks for the detailed review @peixin. I will work on this patch and address comments based on the discussion from patch D116396.

I was planning to add the atomic capture operation like this

// v = x; x = x binop expr;
omp.atomic.capture {
  omp.atomic.read %v = %x : !llvm.ptr<i32>
  omp.atomic.update %x = %x add %expr : !llvm.ptr<i32>, i32
}

This cannot be done with the current syntax because the read value will be limited by scope. All the forms of atomic capture constructs can be captured like this (with certain restrictions). This will make it less complicated. Also, the `equal

Added check for not allowing same values of src and dest.

Hi @peixin. Thanks for the patch! I had a few comments.

As this has been merged already, I will change those specific points in a separate patch.

Hey folks, this commit seems to cause multiple buildbot failures 1 2 3. Can someone please revert this change while it gets fixed? Thanks!

@ftynse @kiranchandramohan there are unrelated build failures (and this is happening for other patches too). Looks like there is an issue with the build job. Should I land this change without waiting for the build jobs to be rectified?

Thanks for the review @Mogball. Addressed Comments.

Rebase with main

Hey folks, it looks like this commit has caused the buildbot here to run into failures. Can someone please revert this change, fix it and then commit it again?

Rebase with main. (Unrelated build failure)

Rebase with main. Addressed comment. Sure @kiranchandramohan. Will address if there are any post-commit reviews.

Thanks for the review @Mogball and @rriddle. I have tried to address all your comments.

Addressed comments. Thanks for the review!

Thanks for the explanation!

Hi @peixin. Thanks for the patch. I had a few comments.

Thanks for the review. Rebase with main.

Thanks for the detailed review @Mogball. I have tried to address all your comments. Please let me know if I missed something!

The patch functionally LGTM, but I have not worked on these files and would suggest you to wait for someone else's review (someone who has worked on semantic checks for flang) before merging this.

Addressed comments

Remove unrelated change

Added new line at EOF

Thanks for the review @NimishMishra.

Hi @peixin . Thanks for this patch. I have a few comments and questions for the declare target checks. I will review the threadprivate checks soon.

Thanks for the review @ftynse. Addressed comments. Waiting for review on OpenMP Part.

Thanks for the patch Nimish. I had a few minor comments about testcases. Rest LGTM.

Hi @abidmalikwaterloo Thanks for this patch. There are a couple of issues with the styling which is causing the builds to fail. For example trailing spaces, 80-character line wraps and maybe some indentation. I have a few comments.

Thanks for the review @Meinersbur . Rebase with main.

Addressed Comments.

Thanks for the review @kiranchandramohan.

Rebase with main

Modified the other test case for shiftr too (missed in previous diff)

Rebase with main, addressed comments

Rebase with main. Addressed comments.

Ping!

Changed argument name in comments and arguments. This is ready for review now.

Removed extra return in one test case (was added by mistake during rebase)

Address comments and rebase with main

Thanks for the comments @peixin! I have tried to address them all, and will include the comments as mentioned if it is okay with you.

Rebase with main

Thanks for this patch. I have a few comments @Leporacanthicus.

Addressed comments, rebase with main