This patch adds the codegen support for atomic compare in clang.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
clang/test/OpenMP/atomic_compare_codegen.cpp | ||
---|---|---|
1 | The test case itself was generated from a Python script. | |
3 | This test cannot be done because clang crashed. I filed a bug https://github.com/llvm/llvm-project/issues/53601. | |
6 | CodeGen for -fopen-simd is wrong. It doesn't emit corresponding atomic operations. I'll fix it. | |
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | ||
3496 | This change will be separated. Actually I got a question about whether we want support equality comparison for floating point values. In the patch, we don't support '<' or '>' for floating point values because we don't have corresponding atomic instruction for them. For '==', we cast them to integers and then compare the two integers. However, We all know it doesn't make too much sense to compare two floating point variables, such as x == d. The comparison of floating point variables should always do like x - d < epsilon. However, like we said before, that comparison cannot be done because of lack of instruction. So I'm not sure if we want to support floating point variables at all. |
clang/lib/CodeGen/CGStmtOpenMP.cpp | ||
---|---|---|
6041 | 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. |
clang/test/OpenMP/atomic_compare_codegen.cpp | ||
---|---|---|
6 | The logic behind -fopenmp-simd is to emit the code only for simd specific instructions. |
clang/test/OpenMP/atomic_compare_codegen.cpp | ||
---|---|---|
6 | Right. // SIMD-ONLY0-NOT: {{__kmpc|__tgt}} is used here, same as other cases. |
clang/test/OpenMP/atomic_compare_codegen.cpp | ||
---|---|---|
1991 | Can we somehow avoid the load here? |
clang/test/OpenMP/atomic_compare_codegen.cpp | ||
---|---|---|
1991 | Do you need EmitScalarExpr? |
clang/test/OpenMP/atomic_compare_codegen.cpp | ||
---|---|---|
1991 | So it is a rvalue scalar here. What alternative do we have then? |
clang/lib/CodeGen/CGStmtOpenMP.cpp | ||
---|---|---|
6041 | I have a better explain in another inline comment in Sema. | |
clang/lib/Sema/SemaOpenMP.cpp | ||
11180 | Here we do D->IgnoreImpCasts(), as shown in the code. The reason is, if we have two chars, 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. 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. |
clang/test/OpenMP/atomic_compare_codegen.cpp | ||
---|---|---|
1991 | I have to get it passed to IRBuilder, which accepts a llvm::Value *. |
clang/test/OpenMP/atomic_compare_codegen.cpp | ||
---|---|---|
1991 | Modify irbuilder to not require it in some cases. |
clang/test/OpenMP/atomic_compare_codegen.cpp | ||
---|---|---|
1991 | 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. |
Fix comment