ABataev added inline comments.
================
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:
> ABataev wrote:
> > tianshilei1992 wrote:
> > > 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.
> > Sorry, did not understand it was a question. Could you provide a bit more
> > background, did not quite understand problem?
> I have a better explain in another inline comment in Sema.
If you don't need it, do not call it. Why do you need this rvalue?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:11231
C = Cond;
- D = CO->getTrueExpr();
+ D = CO->getTrueExpr()->IgnoreImpCasts();
if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS())) {
----------------
tianshilei1992 wrote:
> Here we do `D->IgnoreImpCasts()`, as shown in the code. The reason is, if we
> have two `char`s, say `char a, b`, and when `a > b`, clang also inserts an
> implicit cast from `char` to `int` for `a` and `b`. We want the actual type
> here otherwise in the IR builder, the type of `D` doesn't match `X`'s,
> causing issues.
> However, that `IgnoreImpCasts` here can cause another issue. Let's say we
> have the following code:
> ```
> int x;
> #pragma omp atomic compare
> x = x > 1.01 ? 1.01 : x;
> ```
> clang will also insert an implicit cast from floating point value to integer
> for the scalar value `1.01` (corresponding to `D` in the code), and also
> emits a warning saying the floating point value has been cast to integer or
> something like that. Because we have the `IgnoreImpCasts` now, it will fail
> the type check because `D` now is of floating point type, and we will emit an
> error to user.
> For users, it might be confusing because on one hand we emit a warning saying
> the floating point value has been casted, and on the other hand in Sema we
> tell users it is expected an integer. Users might be thinking, you already
> cast it to integer, why do you tell me it's a floating point value again?
Yiu have the resulting type from the whole expression. You van do explicit cast
to this resulting type to avoid problems, if I got it correctly
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
