tianshilei1992 marked an inline comment as done.
tianshilei1992 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;
+
----------------
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.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:11231
C = Cond;
- D = CO->getTrueExpr();
+ D = CO->getTrueExpr()->IgnoreImpCasts();
if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS())) {
----------------
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?
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
