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

Shilei Tian via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 6 18:09:01 PST 2022


tianshilei1992 added inline comments.


================
Comment at: clang/test/OpenMP/atomic_compare_codegen.cpp:1
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex "__omp_offloading_[0-9a-z]+_[0-9a-z]+" "reduction_size[.].+[.]" "pl_cond[.].+[.|,]" --prefix-filecheck-ir-name _
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 -fopenmp -fopenmp-version=51 -x c -emit-llvm %s -o - | FileCheck %s
----------------
The test case itself was generated from a Python script.


================
Comment at: clang/test/OpenMP/atomic_compare_codegen.cpp:3
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 -fopenmp -fopenmp-version=51 -x c -emit-llvm %s -o - | FileCheck %s
+// %clang_cc1 -fopenmp -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 -target-cpu core2 -emit-pch -o %t %s
+// %clang_cc1 -fopenmp -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 -target-cpu core2 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
----------------
This test cannot be done because clang crashed. I filed a bug https://github.com/llvm/llvm-project/issues/53601.


================
Comment at: clang/test/OpenMP/atomic_compare_codegen.cpp:6
+
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -target-cpu core2 -fopenmp-simd -fopenmp-version=51 -x c -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// %clang_cc1 -fopenmp-simd -fopenmp-version=51 -x c -triple x86_64-apple-darwin10 -target-cpu core2 -emit-pch -o %t %s
----------------
CodeGen for `-fopen-simd` is wrong. It doesn't emit corresponding atomic operations. I'll fix it.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3496
             : Builder.CreateBitCast(X.Var, IntCastTy->getPointerTo(Addrspace));
+    Value *NewE = E->getType()->isFloatingPointTy()
+                      ? Builder.CreateFPToSI(E, IntCastTy)
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118632



More information about the llvm-commits mailing list