[PATCH] D127803: Generate the capture for field when the field is used in openmp region with implicit default in the member function.

[mgabka via Phabricator via cfe-commits]

mgabka added a comment.

I noticed that this patch is causing now an assertion failure for cases like :

class A{

  void a() {
#pragma omp parallel
a(b);
  }

};

The failed assertion is: "const clang::ValueDecl* getCanonicalDecl(const 
clang::ValueDecl*): Assertion `FD' failed."

while before it clang was correctly  reporting error:
error: use of undeclared identifier 'b'

is it the same assertion you were trying to fix here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127803

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127803: Generate the capture for field when the field is used in openmp region with implicit default in the member function.

[mgabka via Phabricator via cfe-commits]

mgabka added a comment.

In D127803#3693700 , @jyu2 wrote:

> In D127803#3693660 , @ABataev wrote:
>
>> In D127803#3693655 , @jyu2 wrote:
>>
>>> In D127803#3693301 , @mgabka 
>>> wrote:
>>>
 I noticed that this patch is causing now an assertion failure for cases 
 like :

 class A{

   void a() {
 #pragma omp parallel
 a(b);
   }

 };

 The failed assertion is: "const clang::ValueDecl* getCanonicalDecl(const 
 clang::ValueDecl*): Assertion `FD' failed."

 while before it clang was correctly  reporting error:
 error: use of undeclared identifier 'b'

 is it the same assertion you were trying to fix here?
>>>
>>> No, the assert I am fixing is when default(firstprivate) is used inside 
>>> member function.
>>
>> Could you please investigate and fix it?
>
> Yes, I will fix this.

Thanks a lot, I raised an upstream bug for it 
https://github.com/llvm/llvm-project/issues/56884


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127803

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131580: [clang][SVE] Undefine preprocessor macro defined in

[mgabka via Phabricator via cfe-commits]

mgabka created this revision.
mgabka added a reviewer: paulwalker-arm.
Herald added subscribers: ctetreau, psnobl, kristof.beyls, tschuett.
Herald added a reviewer: efriedma.
Herald added a project: All.
mgabka requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

arm_sve.h defines and uses __ai macro which needs to be undefined (as it is
already in arm_neon.h).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131580

Files:
  clang/utils/TableGen/SveEmitter.cpp


Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -1282,6 +1282,7 @@
   OS << "#ifdef __cplusplus\n";
   OS << "} // extern \"C\"\n";
   OS << "#endif\n\n";
+  OS << "#undef __ai\n\n";
   OS << "#endif /*__ARM_FEATURE_SVE */\n\n";
   OS << "#endif /* __ARM_SVE_H */\n";
 }


Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -1282,6 +1282,7 @@
   OS << "#ifdef __cplusplus\n";
   OS << "} // extern \"C\"\n";
   OS << "#endif\n\n";
+  OS << "#undef __ai\n\n";
   OS << "#endif /*__ARM_FEATURE_SVE */\n\n";
   OS << "#endif /* __ARM_SVE_H */\n";
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113776: [Clang][SVE] Properly enable/disable dependant SVE target features based upon +(no)sve.* options

[mgabka via Phabricator via cfe-commits]

mgabka added inline comments.



Comment at: clang/test/Driver/aarch64-implied-sve-features.c:28
+
+// RUN: %clang -target aarch64-linux-gnu -march=armv8-a+nosve2-bitperm %s -### 
|& FileCheck %s --check-prefix=NOSVE2-BITPERM
+// NOSVE2-BITPERM-NOT: "-target-feature" "+sve2-bitperm"

Hi,
@bsmith could you at at least one test for +nosve2-sha3, +nosve2-aes and 
+nosve2-sm4?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113776

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131580: [clang][SVE] Undefine preprocessor macro defined in

[mgabka via Phabricator via cfe-commits]

mgabka updated this revision to Diff 451791.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131580

Files:
  clang/utils/TableGen/SveEmitter.cpp


Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -1282,6 +1282,8 @@
   OS << "#ifdef __cplusplus\n";
   OS << "} // extern \"C\"\n";
   OS << "#endif\n\n";
+  OS << "#undef __ai\n\n";
+  OS << "#undef __aio\n\n";
   OS << "#endif /*__ARM_FEATURE_SVE */\n\n";
   OS << "#endif /* __ARM_SVE_H */\n";
 }


Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -1282,6 +1282,8 @@
   OS << "#ifdef __cplusplus\n";
   OS << "} // extern \"C\"\n";
   OS << "#endif\n\n";
+  OS << "#undef __ai\n\n";
+  OS << "#undef __aio\n\n";
   OS << "#endif /*__ARM_FEATURE_SVE */\n\n";
   OS << "#endif /* __ARM_SVE_H */\n";
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131580: [clang][SVE] Undefine preprocessor macro defined in

[mgabka via Phabricator via cfe-commits]

mgabka marked an inline comment as done.
mgabka added inline comments.



Comment at: clang/utils/TableGen/SveEmitter.cpp:1285
   OS << "#endif\n\n";
+  OS << "#undef __ai\n\n";
   OS << "#endif /*__ARM_FEATURE_SVE */\n\n";

paulwalker-arm wrote:
> Can you also do this for `__aio`?
sure, I didn't notice that one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131580

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131580: [clang][SVE] Undefine preprocessor macro defined in

[mgabka via Phabricator via cfe-commits]

mgabka marked an inline comment as done.
mgabka added a comment.

It is a fix for https://github.com/llvm/llvm-project/issues/57083


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131580

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126479: [Clang] Allow 'Complex float __attribute__((mode(HC)))'

[mgabka via Phabricator via cfe-commits]

mgabka added inline comments.



Comment at: clang/test/CodeGen/aarch64-attr-mode-complex.c:11
+// CHECK: define{{.*}} { half, half } @c16_test([2 x half] noundef 
[[C16A:%.*]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   [[C16B:%.*]] = extractvalue [2 x half] [[C16A]], 0

Hi Jolanta,
I the tests from this file, I don't think these check lines are necessary, if 
it was me I would rather check only the type of the argument passed into the 
function and also the return type.
If you did it like that you could also remove the -Ofast from the RUN lines, as 
it does not have impact on the functionality you are testing here.




Comment at: clang/test/Sema/attr-mode.c:40
 
+typedef _Complex float c16a __attribute((mode(HC)));
+int c16a_test[sizeof(c16a) == 4 ? 1 : -1];

shouldn't we have here and in the line below : "// expected-no-diagnostics" ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126479

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159361: [Driver] Do not generate error about unsupported target specific options when there is no compiler jobs

[mgabka via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9272aa9d08cb: [Driver] Do not generate error about 
unsupported target specific options when… (authored by mgabka).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159361

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/no-action.c


Index: clang/test/Driver/no-action.c
===
--- /dev/null
+++ clang/test/Driver/no-action.c
@@ -0,0 +1,10 @@
+// RUN: %clang --target=aarch64-none-gnu --verbose -mcpu= -march= 2>&1 | 
FileCheck %s --check-prefix=WARNING
+// RUN: %clang --target=aarch64-none-gnu -### -mcpu= -march= 2>&1 | FileCheck 
%s --check-prefix=WARNING
+
+// RUN: %clang --target=x86_64-unknown-linux-gnu --verbose -mcpu= -march= 2>&1 
| FileCheck %s --check-prefix=WARNING
+// RUN: %clang --target=x86_64-unknown-linux-gnu -### -mcpu= -march= 2>&1 | 
FileCheck %s --check-prefix=WARNING
+
+/// In situation when there is no compilation/linking clang should not emit 
error
+/// about target specific options, but just warn that are not used.
+WARNING: warning: argument unused during compilation
+WARNING: warning: argument unused during compilation
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4953,7 +4953,12 @@
   // already been warned about.
   if (!IsCLMode() || !A->getOption().matches(options::OPT_UNKNOWN)) {
 if (A->getOption().hasFlag(options::TargetSpecific) &&
-!A->isIgnoredTargetSpecific() && !HasAssembleJob) {
+!A->isIgnoredTargetSpecific() && !HasAssembleJob &&
+// When for example -### or -v is used
+// without a file, target specific options are not
+// consumed/validated.
+// Instead emitting an error emit a warning instead.
+!C.getActions().empty()) {
   Diag(diag::err_drv_unsupported_opt_for_target)
   << A->getSpelling() << getTargetTriple();
 } else {


Index: clang/test/Driver/no-action.c
===
--- /dev/null
+++ clang/test/Driver/no-action.c
@@ -0,0 +1,10 @@
+// RUN: %clang --target=aarch64-none-gnu --verbose -mcpu= -march= 2>&1 | FileCheck %s --check-prefix=WARNING
+// RUN: %clang --target=aarch64-none-gnu -### -mcpu= -march= 2>&1 | FileCheck %s --check-prefix=WARNING
+
+// RUN: %clang --target=x86_64-unknown-linux-gnu --verbose -mcpu= -march= 2>&1 | FileCheck %s --check-prefix=WARNING
+// RUN: %clang --target=x86_64-unknown-linux-gnu -### -mcpu= -march= 2>&1 | FileCheck %s --check-prefix=WARNING
+
+/// In situation when there is no compilation/linking clang should not emit error
+/// about target specific options, but just warn that are not used.
+WARNING: warning: argument unused during compilation
+WARNING: warning: argument unused during compilation
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4953,7 +4953,12 @@
   // already been warned about.
   if (!IsCLMode() || !A->getOption().matches(options::OPT_UNKNOWN)) {
 if (A->getOption().hasFlag(options::TargetSpecific) &&
-!A->isIgnoredTargetSpecific() && !HasAssembleJob) {
+!A->isIgnoredTargetSpecific() && !HasAssembleJob &&
+// When for example -### or -v is used
+// without a file, target specific options are not
+// consumed/validated.
+// Instead emitting an error emit a warning instead.
+!C.getActions().empty()) {
   Diag(diag::err_drv_unsupported_opt_for_target)
   << A->getSpelling() << getTargetTriple();
 } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144179: [Clang] Added functionality to provide config file name via env variable

[mgabka via Phabricator via cfe-commits]

mgabka added a comment.

In D144179#4146599 , @MaskRay wrote:

> This looks like introducing a footgun (when something behaves differently 
> from an upstream Clang, it would be difficult for toolchain maintainers to 
> know why).
> Why can't your user specify `CLANG_CONFIG_FILE_SYSTEM_DIR`?

hi @MaskRay,
The reason we wanted to suggest use of environment variable is that the 
CLANG_CONFIG_FILE_SYSTEM_DIR is only defined at compilation time, after 
discussing it once again we would rather lean towards introducing an 
environment variable with similar semantics as CLANG_CONFIG_FILE_SYSTEM_DIR or 
rather ``CLANG_CONFIG_FILE_USER_DIR``, the motivation here is that it will 
allow to specify the directory to search for config files in a dynamic way, 
without need to recompile the compiler.
It is for user convenience in situations when they are using a system wide 
installation in a location where they do not have access right, and the 
``CLANG_CONFIG_FILE_SYSTEM_DIR`` and ``CLANG_CONFIG_FILE_USER_DIR`` were not 
defined at build time.

We realised that environment variables are already used in this area, for 
example CLANG_NO_DEFAULT_CONFIG, so adding another one is not breaking existing 
convention.

What do you think about it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144179

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150553: [SVE ACLE] Change the lowering of SVE integer mla_u/mls_u builtins

[mgabka via Phabricator via cfe-commits]

mgabka added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:1665
 def int_aarch64_sve_mla_lane   : AdvSIMD_3VectorArgIndexed_Intrinsic;
+def int_aarch64_sve_mla_u  : AdvSIMD_Pred3VectorArg_Intrinsic;
 def int_aarch64_sve_mls: AdvSIMD_Pred3VectorArg_Intrinsic;

it is not a bug, but just a preference to keep better ordering here, could you 
move this new definition to be straight below the def int_aarch64_sve_mla ? so 
the regular and _u intrinsics are defined in the same order as in all other 
cases.

Same request to the mls_u definition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150553

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits