[PATCH] D118632: [Clang][OpenMP] Add the codegen support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.


================
Comment at: clang/test/OpenMP/atomic_compare_codegen.cpp:1990
+// CHECK-NEXT:    [[DD:%.*]] = alloca double, align 8
+// CHECK-NEXT:    [[TMP0:%.*]] = load i8, i8* [[CE]], align 1
+// CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw umin i8* [[CX]], i8 [[TMP0]] monotonic, align 1
----------------
ABataev wrote:
> tianshilei1992 wrote:
> > tianshilei1992 wrote:
> > > ABataev wrote:
> > > > tianshilei1992 wrote:
> > > > > ABataev wrote:
> > > > > > tianshilei1992 wrote:
> > > > > > > tianshilei1992 wrote:
> > > > > > > > tianshilei1992 wrote:
> > > > > > > > > I think the `store` here is redundant. Is it because I'm using `CGF.EmitScalarExpr`?
> > > > > > > > Oh, shoot. `load` here, instead of `store`.
> > > > > > > And here. @ABataev 
> > > > > > Yes, because of EmitScalarExpr
> > > > > Can we somehow avoid the `load` here?
> > > > Do you need EmitScalarExpr?
> > > So it is a rvalue scalar here. What alternative do we have then?
> > I have to get it passed to IRBuilder, which accepts a `llvm::Value *`.
> Modify irbuilder to not require it in some cases.
You got me wrong. I need to call a function to convert a rvalue `Express *` to `llvm::Value *`. `CGF.EmitScalarExpr` can do it. Actually now I feel the `load` here can not be avoided as we want a scalar value instead of a pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118632