tianshilei1992 added a comment. In D118632#3336145 <https://reviews.llvm.org/D118632#3336145>, @ABataev wrote:
> In D118632#3336133 <https://reviews.llvm.org/D118632#3336133>, > @tianshilei1992 wrote: > >> In D118632#3336094 <https://reviews.llvm.org/D118632#3336094>, @ABataev >> wrote: >> >>> LG with a nit >> >> Actually I got two question in the inline comments. Can you please take a >> look? > > Could you point me? Sure. ================ Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6041 + llvm::Value *EVal = CGF.EmitScalarExpr(E->IgnoreImpCasts()); + llvm::Value *DVal = D ? CGF.EmitScalarExpr(D->IgnoreImpCasts()) : nullptr; + ---------------- tianshilei1992 wrote: > Using `D->IgnoreImpCasts()` can make sure to avoid the case that `char` is > casted to `int` in binary operation. However, say, if user writes the > following code: > ``` > int x; > #pragma omp atomic compare > x = x > 1.01 ? 1.01 : x; > ``` > `1.01` here will be casted to `1` by clang, and a warning will be emitted. > Because we ignore the implicit cast, in Sema, it is taken as floating point > value. However, we already told user that it is casted to `1`, which is a > little weird to emit an error then. @ABataev Here. ================ 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 ---------------- 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 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118632/new/ https://reviews.llvm.org/D118632 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits