[PATCH] D107420: [WIP][sema] Disallow __builtin_mul_overflow under special condition.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe updated this revision to Diff 363991.
FreddyYe added a comment.

rebase and refactor clang-format and clang-tidy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107420

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/builtins-overflow.c


Index: clang/test/Sema/builtins-overflow.c
===
--- clang/test/Sema/builtins-overflow.c
+++ clang/test/Sema/builtins-overflow.c
@@ -38,4 +38,10 @@
 _ExtInt(129) result;
 _Bool status = __builtin_mul_overflow(x, y, &result); // expected-error 
{{__builtin_mul_overflow does not support signed _ExtInt operands of more than 
128 bits}}
   }
+  {
+_ExtInt(128) x = 1;
+_ExtInt(128) y = 1;
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, &result); // expected-error 
{{__builtin_mul_overflow does not support special combination operands (signed, 
signed, unsigned*) of more than 64 bits}}
+  }
 }
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -328,6 +328,21 @@
   // Disallow signed ExtIntType args larger than 128 bits to mul function until
   // we improve backend support.
   if (BuiltinID == Builtin::BI__builtin_mul_overflow) {
+const auto LeftTy = TheCall->getArg(0)->getType();
+const auto RightTy = TheCall->getArg(1)->getType();
+const auto ResultTy = TheCall->getArg(2)->getType()->getPointeeType();
+// Input compination below will also emit integer value larger than
+// 128 bits in backend, disallow same as above.
+if (LeftTy->isSignedIntegerType() &&
+S.getASTContext().getIntWidth(LeftTy) > 64 &&
+RightTy->isSignedIntegerType() &&
+S.getASTContext().getIntWidth(RightTy) > 64 &&
+!ResultTy->isSignedIntegerType() &&
+S.getASTContext().getIntWidth(ResultTy) > 64) {
+  return S.Diag(TheCall->getArg(0)->getBeginLoc(),
+diag::err_overflow_builtin_special_combination_max_size)
+ << 64;
+}
 for (unsigned I = 0; I < 3; ++I) {
   const auto Arg = TheCall->getArg(I);
   // Third argument will be a pointer.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8348,6 +8348,9 @@
 def err_overflow_builtin_ext_int_max_size : Error<
   "__builtin_mul_overflow does not support signed _ExtInt operands of more "
   "than %0 bits">;
+def err_overflow_builtin_special_combination_max_size : Error<
+  "__builtin_mul_overflow does not support special combination operands 
(signed, signed, unsigned*) "
+  "of more than %0 bits">;
 
 def err_atomic_load_store_uses_lib : Error<
   "atomic %select{load|store}0 requires runtime support that is not "


Index: clang/test/Sema/builtins-overflow.c
===
--- clang/test/Sema/builtins-overflow.c
+++ clang/test/Sema/builtins-overflow.c
@@ -38,4 +38,10 @@
 _ExtInt(129) result;
 _Bool status = __builtin_mul_overflow(x, y, &result); // expected-error {{__builtin_mul_overflow does not support signed _ExtInt operands of more than 128 bits}}
   }
+  {
+_ExtInt(128) x = 1;
+_ExtInt(128) y = 1;
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, &result); // expected-error {{__builtin_mul_overflow does not support special combination operands (signed, signed, unsigned*) of more than 64 bits}}
+  }
 }
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -328,6 +328,21 @@
   // Disallow signed ExtIntType args larger than 128 bits to mul function until
   // we improve backend support.
   if (BuiltinID == Builtin::BI__builtin_mul_overflow) {
+const auto LeftTy = TheCall->getArg(0)->getType();
+const auto RightTy = TheCall->getArg(1)->getType();
+const auto ResultTy = TheCall->getArg(2)->getType()->getPointeeType();
+// Input compination below will also emit integer value larger than
+// 128 bits in backend, disallow same as above.
+if (LeftTy->isSignedIntegerType() &&
+S.getASTContext().getIntWidth(LeftTy) > 64 &&
+RightTy->isSignedIntegerType() &&
+S.getASTContext().getIntWidth(RightTy) > 64 &&
+!ResultTy->isSignedIntegerType() &&
+S.getASTContext().getIntWidth(ResultTy) > 64) {
+  return S.Diag(TheCall->getArg(0)->getBeginLoc(),
+diag::err_overflow_builtin_special_combination_max_size)
+ << 64;
+}
 for (unsigned I = 0; I < 3; ++I) {
   c

[PATCH] D99353: [driver] Make `clang` warn rather then error on `flang` options

[Joachim Protze via Phabricator via cfe-commits]

protze.joachim added a comment.

The use case, where I see the current behavior as an issue is linking multiple 
object files compiled from different languages.
Many build systems will pass CFLAGS as well as FFLAGS to the linker command in 
such case. To automatically link the necessary language-specific runtime 
libraries, I see using the compiler for linking as common practice:

  $(FC) -c fortran-code.F90 $(FFLAGS)
  $(CXX) -c cpp-code.cpp $(CFLAGS)
  $(CC) -c c-code.c $(CFLAGS)
  $(CXX) fortran-code.o cpp-code.o c-code.o $(FFLAGS) $(CFLAGS) 
-l$(FORTRAN_RUNTIME)

To support such workflow, the compiler should not error out, at least when only 
used for linking.

This works with `CC=gcc`, `CXX=g++`, `FC=gfortran`, and 
`FORTRAN_RUNTIME=gfortran`.
This used to work with `CC=clang`, `CXX=clang++`, `FC=gfortran`, and 
`FORTRAN_RUNTIME=gfortran`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99353

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

[PATCH] D106701: [clang] Add -falign-loops=N where N is a power of 2

[Luís Marques via Phabricator via cfe-commits]

luismarques accepted this revision.
luismarques added a comment.
This revision is now accepted and ready to land.

Still LGTM.

BTW, I liked that in the old version the help string included "In GCC =0 is the 
same as =1".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106701

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

[PATCH] D107433: [RISCV] Half-precision for vget/vset.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai created this revision.
HsiangKai added reviewers: craig.topper, rogfer01, frasercrmck, khchen.
Herald added subscribers: StephenFan, vkmr, evandro, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, 
niosHD, sabuasal, simoncook, johnrusso, rbar, asb.
HsiangKai requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107433

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vset.c

Index: clang/test/CodeGen/RISCV/rvv-intrinsics/vset.c
===
--- clang/test/CodeGen/RISCV/rvv-intrinsics/vset.c
+++ clang/test/CodeGen/RISCV/rvv-intrinsics/vset.c
@@ -1,6 +1,7 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // REQUIRES: riscv-registered-target
-// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -target-feature +experimental-v \
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d \
+// RUN:   -target-feature +experimental-v -target-feature +experimental-zfh \
 // RUN:   -disable-O0-optnone -emit-llvm %s -o - | opt -S -mem2reg | FileCheck --check-prefix=CHECK-RV64 %s
 
 #include 
@@ -544,3 +545,57 @@
 vfloat64m8_t test_vset_v_f64m4_f64m8(vfloat64m8_t dest, vfloat64m4_t val) {
   return vset_v_f64m4_f64m8(dest, 1, val);
 }
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m1_f16m2(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv8f16.nxv4f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m2_t test_vset_v_f16m1_f16m2 (vfloat16m2_t dest, size_t index, vfloat16m1_t val) {
+  return vset_v_f16m1_f16m2(dest, 0, val);
+}
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m1_f16m4(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv16f16.nxv4f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m4_t test_vset_v_f16m1_f16m4 (vfloat16m4_t dest, size_t index, vfloat16m1_t val) {
+  return vset_v_f16m1_f16m4(dest, 0, val);
+}
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m2_f16m4(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv16f16.nxv8f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m4_t test_vset_v_f16m2_f16m4 (vfloat16m4_t dest, size_t index, vfloat16m2_t val) {
+  return vset_v_f16m2_f16m4(dest, 0, val);
+}
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m1_f16m8(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv32f16.nxv4f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m8_t test_vset_v_f16m1_f16m8 (vfloat16m8_t dest, size_t index, vfloat16m1_t val) {
+  return vset_v_f16m1_f16m8(dest, 0, val);
+}
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m2_f16m8(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv32f16.nxv8f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m8_t test_vset_v_f16m2_f16m8 (vfloat16m8_t dest, size_t index, vfloat16m2_t val) {
+  return vset_v_f16m2_f16m8(dest, 0, val);
+}
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m4_f16m8(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv32f16.nxv16f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m8_t test_vset_v_f16m4_f16m8 (vfloat16m8_t dest, size_t index, vfloat16m4_t val) {
+  return vset_v_f16m4_f16m8(dest, 0, val);
+}
Index: clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
===
--- clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
+++ clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
@@ -1,6 +1,7 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // REQUIRES: riscv-registered-target
-// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -target-feature +experimental-v \
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d \
+// RUN:   -target-feature +experimental-v -target-feature +experimental-zfh \
 // RUN:   -disable-O0-optnone -emit-llvm %s -o - | opt -S -mem2reg | FileCheck --check-prefix=CHECK-RV64 %s
 
 #include 
@@ -544,3 +545,57 @@
 vfloat64m4_t test_vget_v_f64m8_f64m4(vfloat64m8_t src) {
   return vget_v_f64m8_f64m4(src, 1);
 }
+
+// CHECK-RV64-LABEL: @test_vget_v_f16m2_f16m1(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.extract.nxv4f16.nxv8f16(

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

[Hans Wennborg via Phabricator via cfe-commits]

hans added a subscriber: tstellar.
hans added a comment.

Also +@tstellar since I believe this went in before the 13 branch point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74436

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

[PATCH] D106823: [analyzer][solver] Iterate to a fixpoint during symbol simplification with constants

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping. @vsavchenko Could you please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106823

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

[PATCH] D106823: [analyzer][solver] Iterate to a fixpoint during symbol simplification with constants

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Looking great!
I have a couple of nit picks and I kind of want to check that it doesn't affect 
the performance on a different set of projects as well.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2112-2113
+  assert(ClsMembers.contains(Old));
+  assert(ClsMembers.getHeight() > 1 &&
+ "Class should have at least two members");
+

Maybe after removing you can check that `ClsMembers` is not empty?
I just don't like relying on `getHeight` because it looks like an 
implementation detail and shouldn't be used.  We use it only in one place in 
the whole codebase (in equivalence classes :) ), but since we can re-write this 
assertion to have a simpler condition, I think that it should be preferred.



Comment at: 
clang/test/Analysis/symbol-simplification-fixpoint-iteration-unreachable-code.cpp:20
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  clang_analyzer_printState();
+

Do we need to print states in this test?



Comment at: 
clang/test/Analysis/symbol-simplification-fixpoint-one-iteration.cpp:32
+  // CHECK-NEXT:   "equivalence_classes": [
+  // CHECK-NEXT: [ "(reg_$0) != (reg_$2)" ],
+  // CHECK-NEXT: [ "reg_$0", "reg_$2" ]

Same question here



Comment at: 
clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp:36
+  // CHECK-NEXT:  "equivalence_classes": [
+  // CHECK-NEXT:[ "(reg_$0) != (reg_$3)" ],
+  // CHECK-NEXT:[ "reg_$0", "reg_$3" ],

OK, I understand the next two classes.
But how did we produce this one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106823

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

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan updated this revision to Diff 364013.
TaoPan added a comment.

Change selection of entry block text section from IMAGE_COMDAT_SELECT_ANY to 
return value of getSelectionForCOFF()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fbasic-block-sections.c
  llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/test/CodeGen/X86/basic-block-sections.ll

Index: llvm/test/CodeGen/X86/basic-block-sections.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections.ll
+++ llvm/test/CodeGen/X86/basic-block-sections.ll
@@ -3,6 +3,10 @@
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=all -unique-basic-block-section-names -split-machine-functions | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-FUNCTION-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-SECTIONS
 
 define void @_Z3bazb(i1 zeroext) nounwind {
   %2 = alloca i8, align 1
@@ -39,3 +43,27 @@
 ; LINUX-SECTIONS: [[SECTION_LABEL_2]]:
 ; LINUX-SECTIONS: .LBB_END0_2:
 ; LINUX-SECTIONS-NEXT: .size   [[SECTION_LABEL_2]], .LBB_END0_2-[[SECTION_LABEL_2]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,_Z3bazb
+; WINDOWS-MSVC-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-MSVC-SECTIONS: .section.text,"xr"
+; WINDOWS-MSVC-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-SECTIONS: je  [[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: jmp [[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text$_Z3bazb,"xr",one_only,_Z3bazb
+; WINDOWS-GNU-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-SECTIONS: .section.text$_Z3bazb,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-SECTIONS: .section.text,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb.__part.1:
+; WINDOWS-GNU-SECTIONS: .section.text,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb.__part.2:
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1733,6 +1733,65 @@
   COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE, UniqueID);
 }
 
+/// Returns a unique section for the given machine basic block.
+MCSection *TargetLoweringObjectFileCOFF::getSectionForMachineBasicBlock(
+const Function &F, const MachineBasicBlock &MBB,
+const TargetMachine &TM) const {
+  assert(MBB.isBeginSection() && "Basic block does not start a section!");
+  SectionKind Kind = SectionKind::getText();
+  unsigned Characteristics = getCOFFSectionFlags(Kind, TM);
+  // If we have -ffunction-sections then we should emit the global value to a
+  // uniqued section specifically for it.
+  if (TM.getFunctionSections() || F.hasComdat())
+Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
+  unsigned UniqueID = MCContext::GenericSectionID;
+  StringRef COMDATSymName;
+  if (TM.getUniqueBasicBlockSectionNames())
+COMDATSymName = MBB.getSymbol()->getName();
+  else {
+Uni

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan added a comment.

@rnk Thanks for your review comments! Could you please help to review my reply 
and new modification?
@MaskRay Could you please also help to review?




Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1712
+  COFF::IMAGE_SCN_MEM_READ | COFF::IMAGE_SCN_LNK_COMDAT,
+  SectionKind::getText(), COMDATSymName,
+  COFF::IMAGE_COMDAT_SELECT_NODUPLICATES, UniqueID);

rnk wrote:
> TaoPan wrote:
> > TaoPan wrote:
> > > TaoPan wrote:
> > > > mstorsjo wrote:
> > > > > rnk wrote:
> > > > > > mstorsjo wrote:
> > > > > > > TaoPan wrote:
> > > > > > > > rnk wrote:
> > > > > > > > > tmsriram wrote:
> > > > > > > > > > MaskRay wrote:
> > > > > > > > > > > TaoPan wrote:
> > > > > > > > > > > > tmsriram wrote:
> > > > > > > > > > > > > COMDATSymName can be folded to be equal to 
> > > > > > > > > > > > > MBB.getSymbol()->getName() here?  Plus, you are not 
> > > > > > > > > > > > > preserving the .text.hot prefix that the original 
> > > > > > > > > > > > > function might get here.  Is this future work?  If 
> > > > > > > > > > > > > the original function is named .text.hot.foo, the 
> > > > > > > > > > > > > basic block will still be named .text.foo.__part.1 
> > > > > > > > > > > > > which is not right.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Plus, what about exception handling sections like 
> > > > > > > > > > > > > ".eh.*"?
> > > > > > > > > > > > Thanks! I'll redesign section name and comdat symbol 
> > > > > > > > > > > > name.
> > > > > > > > > > > > The text section with prefix "hot" and "unlikely" won't 
> > > > > > > > > > > > be constructed here, I added COFF text section prefix 
> > > > > > > > > > > > "hot" and "unlikely" in D92073. In ELF override 
> > > > > > > > > > > > function, also not handling text section with prefix 
> > > > > > > > > > > > "hot" and "unlikely".
> > > > > > > > > > > > The text section with prefix "split" will be 
> > > > > > > > > > > > constructed here, I plan to add related code in MFS 
> > > > > > > > > > > > COFF patch.
> > > > > > > > > > > > Also, exception handling section is future work that 
> > > > > > > > > > > > support basic block sections Windows COFF exception 
> > > > > > > > > > > > handling.
> > > > > > > > > > > This is complex. PE-COFF has multiple COMDAT seletion 
> > > > > > > > > > > kinds. I want to see a holistic plan how every component 
> > > > > > > > > > > is going to be implemented.
> > > > > > > > > > The basic block should just mimic the COMDAT type of its 
> > > > > > > > > > containing function, is there a reason to do anything more 
> > > > > > > > > > with it here?
> > > > > > > > > After thinking about it a bit, I think the entry block should 
> > > > > > > > > use the regular selection kind, and all of the auxilliary MBB 
> > > > > > > > > sections should use IMAGE_COMDAT_SELECT_ASSOCIATIVE. They 
> > > > > > > > > should be associated with the main function symbol, unless 
> > > > > > > > > the function itself is associated with something else, in 
> > > > > > > > > which case the BBs use that symbol.
> > > > > > > > > 
> > > > > > > > > This will ensure that if the main function section prevails, 
> > > > > > > > > they are included, and if it does not prevail, they are 
> > > > > > > > > discarded. Does that make sense?
> > > > > > > > Thanks! I think set entry block sections as regular 
> > > > > > > > IMAGE_COMDAT_SELECT_NODUPLICATES and set the auxilliary MBB 
> > > > > > > > sections as IMAGE_COMDAT_SELECT_ASSOCIATIVE is fine. I changed.
> > > > > > > @rnk - I'm not quite familiar with the concrete implications of 
> > > > > > > this work here, but would this need to be mapped to the GNU style 
> > > > > > > comdats, for compatibility with ld.bfd for mingw targets? (LLD 
> > > > > > > itself works fine with proper associative comdats though.)
> > > > > > I think basic block sections are kind of in the same category as 
> > > > > > LTO: it's a very sophisticated optimization feature, and it's 
> > > > > > probably OK if it's LLD-only. I think it also requires special 
> > > > > > linker support that might make it LLD-only, but I've forgotten the 
> > > > > > details.
> > > > > > 
> > > > > > It also has potentially very high object file size overhead, and it 
> > > > > > will be important to do everything possible to minimize that object 
> > > > > > file size overhead. Long gnu-style section names for every basic 
> > > > > > block section are likely to make the feature unusably slow, so 
> > > > > > maybe it's worth removing these "if mingw" conditionals. We can 
> > > > > > leave behind a comment by the use of 
> > > > > > IMAGE_COMDAT_SELECT_ASSOCIATIVE indicating that gnu-style section 
> > > > > > names are not needed because the feature is only designed to work 
> > > > > > with LLD.
> > > > > Thanks for the clarification! Leaving the feature as LLD-only (or in 
> > > > > other terms, requiring a conforming COMDAT implementation) sounds 

[PATCH] D107430: [OMPIRBuilder] Add ordered without depend and simd clause to OMPBuilder

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

Thanks @peixin for the patch. Could you remove the OpenMP 1.0 target from the 
description? That is a target for the Flang team and since this patch touches 
clang as well, it might be confusing. I believe this is modelling the ordered 
construct with a region. Could simd be another option to CreateOrdered or does 
it need more changes? Also, could you clarify how the standalone ordered 
construct (with depend) will be handled? Will it be an extension to 
CreateOrdered or will be a completely separate function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107430

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

[PATCH] D105819: [analyzer] MallocChecker: Add a visitor to leave a note on functions that could have, but did not change ownership on leaked memory

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Maybe if I added this under an alpha checker option?


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

https://reviews.llvm.org/D105819

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

[PATCH] D106909: [clang] Add clang builtins support for gfx90a

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added a comment.

@rampitec what should I be testing exactly in the IR test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106909

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

[PATCH] D106909: [clang] Add clang builtins support for gfx90a

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

In D106909#2923724 , @gandhi21299 
wrote:

> @rampitec what should I be testing exactly in the IR test?

Produced call to the intrinsic. All of these tests there doing that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106909

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

[PATCH] D106909: [clang] Add clang builtins support for gfx90a

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16270
+llvm::Function *F = CGM.getIntrinsic(IID, {ArgTy});
+return Builder.CreateCall(F, {Addr, Val, ZeroI32, ZeroI32, ZeroI1});
+  }

gandhi21299 wrote:
> rampitec wrote:
> > gandhi21299 wrote:
> > > rampitec wrote:
> > > > Should we map flags since we already have them?
> > > Do you mean the memory order flag?
> > All 3: ordering, scope and volatile.
> Following the discussion, what change is required here?
Keep zeroes, drop immediate argument of the builtins.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106909

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

[PATCH] D106909: [clang] Add clang builtins support for gfx90a

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16270
+llvm::Function *F = CGM.getIntrinsic(IID, {ArgTy});
+return Builder.CreateCall(F, {Addr, Val, ZeroI32, ZeroI32, ZeroI1});
+  }

rampitec wrote:
> gandhi21299 wrote:
> > rampitec wrote:
> > > Should we map flags since we already have them?
> > Do you mean the memory order flag?
> All 3: ordering, scope and volatile.
Following the discussion, what change is required here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106909

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks when an FP atomic instruction is converted into a CAS loop or unsafe hardware instruction for GFX90A

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added a comment.

Passed PSDB, @rampitec


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks when an FP atomic instruction is converted into a CAS loop or unsafe hardware instruction for GFX90A

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

It still does not do anything useful and still produces useless, wrong and 
misleading remarks for all targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

[Anastasiia Lukianenko via Phabricator via cfe-commits]

anastasiia_lukianenko added a comment.

@HazardyKnusperkeks @MyDeveloperDay Could you please review the last version?


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

https://reviews.llvm.org/D91950

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

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Ok if you think that setting GCC version to higher values by default won't 
break anything, I think we should update this for all of clang instead. 
Apparently that was something done in the past already in 
https://github.com/llvm/llvm-project/commit/b5fe494a2cdd33a0b069939795043f538e9a492c.
 It is currently being set in 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L6127.

If we bumped the version number there for all of clang by default, clangd 
wouldn't be lying about the version and this will have a much wider visibility. 
That way we'll be sure that users won't see breakages specific to clangd and 
we'll also be able to get opinions from other stakeholders in clang that know 
about possible side effects of this bump. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107304

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

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:1352-1356
+  // key 'j'
+  if (event.keyCode == 74) {
 navigateTo(/*up=*/false);
-  } else if (event.key == "k") {
+  // key 'k'
+  } else if (event.keyCode == 75) {

NoQ wrote:
> This probably also solves keyboard layout problems, nice!
Exactly=)



Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:1373
+  var array = [];
+  for (var i = lower; i <= upper; ++i) {
+  array.push(i);

NoQ wrote:
> Shouldn't it be `i < upper`?
`[lower, upper).` Sure! Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107366

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

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 364030.
ASDenysPetrov added a comment.

Fixed the range in  `range` function.


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

https://reviews.llvm.org/D107366

Files:
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp


Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -1289,15 +1289,32 @@
 return out;
 };
 
+var classListAdd = function(el, theClass) {
+  if(!el.className.baseVal)
+el.className += " " + theClass;
+  else
+el.className.baseVal += " " + theClass;
+};
+
+var classListRemove = function(el, theClass) {
+  var className = (!el.className.baseVal) ?
+  el.className : el.className.baseVal;
+className = className.replace(" " + theClass, "");
+  if(!el.className.baseVal)
+el.className = className;
+  else
+el.className.baseVal = className;
+};
+
 var scrollTo = function(el) {
 querySelectorAllArray(".selected").forEach(function(s) {
-s.classList.remove("selected");
+  classListRemove(s, "selected");
 });
-el.classList.add("selected");
+classListAdd(el, "selected");
 window.scrollBy(0, el.getBoundingClientRect().top -
 (window.innerHeight / 2));
 highlightArrowsForSelectedEvent();
-}
+};
 
 var move = function(num, up, numItems) {
   if (num == 1 && up || num == numItems - 1 && !up) {
@@ -1332,9 +1349,11 @@
   if (event.defaultPrevented) {
 return;
   }
-  if (event.key == "j") {
+  // key 'j'
+  if (event.keyCode == 74) {
 navigateTo(/*up=*/false);
-  } else if (event.key == "k") {
+  // key 'k'
+  } else if (event.keyCode == 75) {
 navigateTo(/*up=*/true);
   } else {
 return;
@@ -1350,8 +1369,11 @@
 

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:664
+  }
+  // NextPragma is after us but before the next symbol. Our reign is over.
+  break;

I suppose this is not correct in cases like.

```
#pragma mark - Outer
foo {
  #pragma mark - Foo
}
bar {
  # pragma mark - Bar
}
baz {
  # pragma mark - Baz
}
```

We'll first move `foo` under `Outer`, then start processing `bar` we notice 
that It comes after pragma `Foo`, and then also notice that pragma `Foo` is not 
contained in `bar` hence break. but all of `foo,bar,baz` should've been moved 
to be under `Outer` instead.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:535
+/// by range.
+std::vector mergePragmas(std::vector &Syms,
+ std::vector 
&Pragmas,

dgoldman wrote:
> kadircet wrote:
> > dgoldman wrote:
> > > kadircet wrote:
> > > > dgoldman wrote:
> > > > > kadircet wrote:
> > > > > > dgoldman wrote:
> > > > > > > sammccall wrote:
> > > > > > > > FWIW the flow control/how we make progress seem hard to follow 
> > > > > > > > here to me.
> > > > > > > > 
> > > > > > > > In particular I think I'm struggling with the statefulness of 
> > > > > > > > "is there an open mark group".
> > > > > > > > 
> > > > > > > > Possible simplifications:
> > > > > > > >  - define a dummy root symbol, which seems clearer than the 
> > > > > > > > vector + range
> > > > > > > >  - avoid reverse-sorting the list of pragma symbols, and just 
> > > > > > > > consume from the front of an ArrayRef instead
> > > > > > > >  - make the outer loop over pragmas, rather than symbols. It 
> > > > > > > > would first check if the pragma belongs directly here or not, 
> > > > > > > > and if so, loop over symbols to work out which should become 
> > > > > > > > children. This seems very likely to be efficient enough in 
> > > > > > > > practice (few pragmas, or most children are grouped into 
> > > > > > > > pragmas)
> > > > > > > > define a dummy root symbol, which seems clearer than the 
> > > > > > > > vector + range
> > > > > > > 
> > > > > > > I guess? Then we'd take in a `DocumentSymbol & and a 
> > > > > > > ArrayRef & (or just by value and then return it 
> > > > > > > as well). The rest would be the same though
> > > > > > > 
> > > > > > > > In particular I think I'm struggling with the statefulness of 
> > > > > > > > "is there an open mark group".
> > > > > > > 
> > > > > > > We need to track the current open group if there is one in order 
> > > > > > > to move children to it.
> > > > > > > 
> > > > > > > > make the outer loop over pragmas, rather than symbols. It would 
> > > > > > > > first check if the pragma belongs directly here or not, and if 
> > > > > > > > so, loop over symbols to work out which should become children. 
> > > > > > > > This seems very likely to be efficient enough in practice (few 
> > > > > > > > pragmas, or most children are grouped into pragmas)
> > > > > > > 
> > > > > > > The important thing here is knowing where the pragma mark ends - 
> > > > > > > if it doesn't, it actually gets all of the children. So we'd have 
> > > > > > > to peak at the next pragma mark, add all symbols before it to us 
> > > > > > > as children, and then potentially recurse to nest it inside of a 
> > > > > > > symbol. I'll try it out and see if it's simpler.
> > > > > > > 
> > > > > > > 
> > > > > > ```
> > > > > > while(Pragmas) {
> > > > > > // We'll figure out where the Pragmas.front() should go.
> > > > > > Pragma P = Pragmas.front();
> > > > > > DocumentSymbol *Cur = Root;
> > > > > > while(Cur->contains(P)) {
> > > > > >   auto *OldCur = Cur;
> > > > > >   for(auto *C : Cur->children) {
> > > > > >  // We assume at most 1 child can contain the pragma (as 
> > > > > > pragmas are on a single line, and children have disjoint ranges)
> > > > > >  if (C->contains(P)) {
> > > > > >  Cur = C;
> > > > > >  break;
> > > > > >  }
> > > > > >   }
> > > > > >   // Cur is immediate parent of P
> > > > > >   if (OldCur == Cur) {
> > > > > > // Just insert P into children if it is not a group and we are 
> > > > > > done.
> > > > > > // Otherwise we need to figure out when current pragma is 
> > > > > > terminated:
> > > > > > // if next pragma is not contained in Cur, or is contained in one 
> > > > > > of the children, It is at the end of Cur, nest all the children 
> > > > > > that appear after P under the symbol node for P.
> > > > > > // Otherwise nest all the children that appear after P but before 
> > > > > > next pragma under the symbol node for P.
> > > > > > // Pop Pragmas and break
> > > > > >   }
> > > > > > }
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > Does that make sense, i hope i am not missing something obvious? 
> > > > > > Complexity-wise in the worst case we'll go all the way down to a 
> > > > > > leaf once per pragma, since there will only be a han

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Can you please also attach an HTML file just to verify that it works?




Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:1293
+var classListAdd = function(el, theClass) {
+  if(!el.className.baseVal)
+el.className += " " + theClass;

nit: space please



Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:1396
 var arrow = document.querySelector("#arrow" + index);
-arrow.classList.add("selected");
+if(arrow) {
+  classListAdd(arrow, "selected")

nit: space please


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

https://reviews.llvm.org/D107366

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

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

>> I think we should update this for all of clang instead.

But first you need to prove that clang matches gcc x.y.z.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107304

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

[PATCH] D99353: [driver] Make `clang` warn rather then error on `flang` options

[Joachim Protze via Phabricator via cfe-commits]

protze.joachim added a comment.

Since I see this as a regression over clang-12, I posted 
https://bugs.llvm.org/show_bug.cgi?id=51339 as a release blocking issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99353

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

[clang] 0556138 - [clang][cli] Expose -fno-cxx-modules in cc1

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-08-04T13:46:40+02:00
New Revision: 0556138624edf48621dd49a463dbe12e7101f17d

URL: 
https://github.com/llvm/llvm-project/commit/0556138624edf48621dd49a463dbe12e7101f17d
DIFF: 
https://github.com/llvm/llvm-project/commit/0556138624edf48621dd49a463dbe12e7101f17d.diff

LOG: [clang][cli] Expose -fno-cxx-modules in cc1

For some use-cases, it might be useful to be able to turn off modules for C++ 
in `-cc1`. (The feature is implied by `-std=C++20`.)

This patch exposes the `-fno-cxx-modules` option in `-cc1`.

Reviewed By: arphaman

Differential Revision: https://reviews.llvm.org/D106864

Added: 
clang/test/Modules/cxx20-disable.cpp

Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index af01f620a94f9..712aa9f8c5c9c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -467,7 +467,6 @@ defvar render_script = LangOpts<"RenderScript">;
 defvar hip = LangOpts<"HIP">;
 defvar gnu_mode = LangOpts<"GNUMode">;
 defvar asm_preprocessor = LangOpts<"AsmPreprocessor">;
-defvar cpp_modules = LangOpts<"CPlusPlusModules">;
 
 defvar std = !strconcat("LangStandard::getLangStandardForKind(", 
lang_std.KeyPath, ")");
 
@@ -1332,8 +1331,11 @@ defm cxx_exceptions: BoolFOption<"cxx-exceptions",
 defm async_exceptions: BoolFOption<"async-exceptions",
   LangOpts<"EHAsynch">, DefaultFalse,
   PosFlag, 
NegFlag>;
-def fcxx_modules : Flag <["-"], "fcxx-modules">, Group,
-  Flags<[NoXarchOption]>;
+defm cxx_modules : BoolFOption<"cxx-modules",
+  LangOpts<"CPlusPlusModules">, Default,
+  NegFlag, PosFlag,
+  BothFlags<[NoXarchOption], " modules for C++">>,
+  ShouldParseIf;
 def fdebug_pass_arguments : Flag<["-"], "fdebug-pass-arguments">, 
Group;
 def fdebug_pass_structure : Flag<["-"], "fdebug-pass-structure">, 
Group;
 def fdepfile_entry : Joined<["-"], "fdepfile-entry=">,
@@ -2154,7 +2156,7 @@ def fmodules_ts : Flag <["-"], "fmodules-ts">, 
Group,
   Flags<[CC1Option]>, HelpText<"Enable support for the C++ Modules TS">,
   MarshallingInfoFlag>;
 defm modules : BoolFOption<"modules",
-  LangOpts<"Modules">, Default,
+  LangOpts<"Modules">, Default,
   PosFlag,
   NegFlag, BothFlags<[NoXarchOption, CoreOption]>>;
 def fmodule_maps : Flag <["-"], "fmodule-maps">, Flags<[CoreOption]>, 
Alias;
@@ -2213,8 +2215,6 @@ def fno_diagnostics_color : Flag<["-"], 
"fno-diagnostics-color">, Group
   Flags<[CoreOption, NoXarchOption]>;
 def fno_common : Flag<["-"], "fno-common">, Group, Flags<[CC1Option]>,
 HelpText<"Compile common globals like normal definitions">;
-def fno_cxx_modules : Flag <["-"], "fno-cxx-modules">, Group,
-  Flags<[NoXarchOption]>;
 defm digraphs : BoolFOption<"digraphs",
   LangOpts<"Digraphs">, Default,
   PosFlag', 
'<%', '%>', '%:', '%:%:' (default)">,
@@ -5298,7 +5298,7 @@ def fmodules_local_submodule_visibility :
   HelpText<"Enforce name visibility rules across submodules of the same "
"top-level module.">,
   MarshallingInfoFlag>,
-  ImpliedByAnyOf<[fmodules_ts.KeyPath, cpp_modules.KeyPath]>;
+  ImpliedByAnyOf<[fmodules_ts.KeyPath, fcxx_modules.KeyPath]>;
 def fmodules_codegen :
   Flag<["-"], "fmodules-codegen">,
   HelpText<"Generate code for uses of this module that assumes an explicit "

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 39335c4771fa3..2c6ba150f4bc5 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3150,8 +3150,6 @@ void CompilerInvocation::setLangDefaults(LangOptions 
&Opts, InputKind IK,
   Opts.HexFloats = Std.hasHexFloats();
   Opts.ImplicitInt = Std.hasImplicitInt();
 
-  Opts.CPlusPlusModules = Opts.CPlusPlus20;
-
   // Set OpenCL Version.
   Opts.OpenCL = Std.isOpenCL();
   if (LangStd == LangStandard::lang_opencl10)

diff  --git a/clang/test/Modules/cxx20-disable.cpp 
b/clang/test/Modules/cxx20-disable.cpp
new file mode 100644
index 0..d53c6b49ce565
--- /dev/null
+++ b/clang/test/Modules/cxx20-disable.cpp
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: %clang_cc1 -x objective-c++ -std=c++20  -I %t %s 
-verify=enabled
+// RUN: %clang_cc1 -x objective-c++ -std=c++20 -fno-cxx-modules -I %t %s 
-verify=disabled
+
+// enabled-no-diagnostics
+
+// The spelling of these errors is misleading.
+// The important thing is Clang rejected C++20 modules syntax.
+export module Foo; // disabled-error{{expected template}}
+   // disabled-error@-1{{unknown type name 'module'}}



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

[PATCH] D106864: [clang][cli] Expose -fno-cxx-modules in cc1

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0556138624ed: [clang][cli] Expose -fno-cxx-modules in cc1 
(authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106864

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Modules/cxx20-disable.cpp


Index: clang/test/Modules/cxx20-disable.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-disable.cpp
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: %clang_cc1 -x objective-c++ -std=c++20  -I %t %s 
-verify=enabled
+// RUN: %clang_cc1 -x objective-c++ -std=c++20 -fno-cxx-modules -I %t %s 
-verify=disabled
+
+// enabled-no-diagnostics
+
+// The spelling of these errors is misleading.
+// The important thing is Clang rejected C++20 modules syntax.
+export module Foo; // disabled-error{{expected template}}
+   // disabled-error@-1{{unknown type name 'module'}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3150,8 +3150,6 @@
   Opts.HexFloats = Std.hasHexFloats();
   Opts.ImplicitInt = Std.hasImplicitInt();
 
-  Opts.CPlusPlusModules = Opts.CPlusPlus20;
-
   // Set OpenCL Version.
   Opts.OpenCL = Std.isOpenCL();
   if (LangStd == LangStandard::lang_opencl10)
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -467,7 +467,6 @@
 defvar hip = LangOpts<"HIP">;
 defvar gnu_mode = LangOpts<"GNUMode">;
 defvar asm_preprocessor = LangOpts<"AsmPreprocessor">;
-defvar cpp_modules = LangOpts<"CPlusPlusModules">;
 
 defvar std = !strconcat("LangStandard::getLangStandardForKind(", 
lang_std.KeyPath, ")");
 
@@ -1332,8 +1331,11 @@
 defm async_exceptions: BoolFOption<"async-exceptions",
   LangOpts<"EHAsynch">, DefaultFalse,
   PosFlag, 
NegFlag>;
-def fcxx_modules : Flag <["-"], "fcxx-modules">, Group,
-  Flags<[NoXarchOption]>;
+defm cxx_modules : BoolFOption<"cxx-modules",
+  LangOpts<"CPlusPlusModules">, Default,
+  NegFlag, PosFlag,
+  BothFlags<[NoXarchOption], " modules for C++">>,
+  ShouldParseIf;
 def fdebug_pass_arguments : Flag<["-"], "fdebug-pass-arguments">, 
Group;
 def fdebug_pass_structure : Flag<["-"], "fdebug-pass-structure">, 
Group;
 def fdepfile_entry : Joined<["-"], "fdepfile-entry=">,
@@ -2154,7 +2156,7 @@
   Flags<[CC1Option]>, HelpText<"Enable support for the C++ Modules TS">,
   MarshallingInfoFlag>;
 defm modules : BoolFOption<"modules",
-  LangOpts<"Modules">, Default,
+  LangOpts<"Modules">, Default,
   PosFlag,
   NegFlag, BothFlags<[NoXarchOption, CoreOption]>>;
 def fmodule_maps : Flag <["-"], "fmodule-maps">, Flags<[CoreOption]>, 
Alias;
@@ -2213,8 +2215,6 @@
   Flags<[CoreOption, NoXarchOption]>;
 def fno_common : Flag<["-"], "fno-common">, Group, Flags<[CC1Option]>,
 HelpText<"Compile common globals like normal definitions">;
-def fno_cxx_modules : Flag <["-"], "fno-cxx-modules">, Group,
-  Flags<[NoXarchOption]>;
 defm digraphs : BoolFOption<"digraphs",
   LangOpts<"Digraphs">, Default,
   PosFlag', 
'<%', '%>', '%:', '%:%:' (default)">,
@@ -5298,7 +5298,7 @@
   HelpText<"Enforce name visibility rules across submodules of the same "
"top-level module.">,
   MarshallingInfoFlag>,
-  ImpliedByAnyOf<[fmodules_ts.KeyPath, cpp_modules.KeyPath]>;
+  ImpliedByAnyOf<[fmodules_ts.KeyPath, fcxx_modules.KeyPath]>;
 def fmodules_codegen :
   Flag<["-"], "fmodules-codegen">,
   HelpText<"Generate code for uses of this module that assumes an explicit "


Index: clang/test/Modules/cxx20-disable.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-disable.cpp
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: %clang_cc1 -x objective-c++ -std=c++20  -I %t %s -verify=enabled
+// RUN: %clang_cc1 -x objective-c++ -std=c++20 -fno-cxx-modules -I %t %s -verify=disabled
+
+// enabled-no-diagnostics
+
+// The spelling of these errors is misleading.
+// The important thing is Clang rejected C++20 modules syntax.
+export module Foo; // disabled-error{{expected template}}
+   // disabled-error@-1{{unknown type name 'module'}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3150,8 +3150,6 @@
   Opts.HexFloats = Std.hasHexFloats();
   Opts.ImplicitInt = Std.hasImplicitInt();
 
-  Opts.CPlusPlusModules = Opts.CPlusPlus20;
-
   // Set OpenCL Version.
   Opts.OpenCL = Std.isOpenCL();
   

[clang] 2718ae3 - [clang][deps] Substitute clang-scan-deps executable in lit tests

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-08-04T13:55:14+02:00
New Revision: 2718ae397b29f339e65c1e3ca5e834b648732d20

URL: 
https://github.com/llvm/llvm-project/commit/2718ae397b29f339e65c1e3ca5e834b648732d20
DIFF: 
https://github.com/llvm/llvm-project/commit/2718ae397b29f339e65c1e3ca5e834b648732d20.diff

LOG: [clang][deps] Substitute clang-scan-deps executable in lit tests

The lit tests for `clang-scan-deps` invoke the tool without going through the 
substitution system. While the test runner correctly picks up the 
`clang-scan-deps` binary from the build directory, it doesn't print its 
absolute path. When copying the invocations when reproducing test failures, 
this can result in `command not found: clang-scan-deps` errors or worse yet: 
pick up the system `clang-scan-deps`. This patch adds new local 
`%clang-scan-deps` substitution.

Reviewed By: lxfind, dblaikie

Differential Revision: https://reviews.llvm.org/D107155

Added: 


Modified: 
clang/test/lit.cfg.py

Removed: 




diff  --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py
index c872b6aab847..3a820943bd0f 100644
--- a/clang/test/lit.cfg.py
+++ b/clang/test/lit.cfg.py
@@ -64,7 +64,7 @@
 
 tools = [
 'apinotes-test', 'c-index-test', 'clang-
diff ', 'clang-format', 'clang-repl',
-'clang-tblgen', 'opt', 'llvm-ifs', 'yaml2obj',
+'clang-tblgen', 'clang-scan-deps', 'opt', 'llvm-ifs', 'yaml2obj',
 ToolSubst('%clang_extdef_map', command=FindTool(
 'clang-extdef-mapping'), unresolved='ignore'),
 ]



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

[PATCH] D107155: [clang][deps] Substitute clang-scan-deps executable in lit tests

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2718ae397b29: [clang][deps] Substitute clang-scan-deps 
executable in lit tests (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107155

Files:
  clang/test/lit.cfg.py


Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -64,7 +64,7 @@
 
 tools = [
 'apinotes-test', 'c-index-test', 'clang-diff', 'clang-format', 
'clang-repl',
-'clang-tblgen', 'opt', 'llvm-ifs', 'yaml2obj',
+'clang-tblgen', 'clang-scan-deps', 'opt', 'llvm-ifs', 'yaml2obj',
 ToolSubst('%clang_extdef_map', command=FindTool(
 'clang-extdef-mapping'), unresolved='ignore'),
 ]


Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -64,7 +64,7 @@
 
 tools = [
 'apinotes-test', 'c-index-test', 'clang-diff', 'clang-format', 'clang-repl',
-'clang-tblgen', 'opt', 'llvm-ifs', 'yaml2obj',
+'clang-tblgen', 'clang-scan-deps', 'opt', 'llvm-ifs', 'yaml2obj',
 ToolSubst('%clang_extdef_map', command=FindTool(
 'clang-extdef-mapping'), unresolved='ignore'),
 ]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107450: [clang-tidy] Fix wrong and missing warnings in performance-move-const-arg

[gehry via Phabricator via cfe-commits]

Sockke created this revision.
Sockke added reviewers: alexfh, aaron.ballman, whisperity, steven.zhang, MTC.
Herald added subscribers: rnkovacs, xazax.hun.
Sockke requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

There are incorrect Fixit and missing warnings:
case 1:
A trivially-copyable object wrapped by std::move is passed to the function with 
rvalue reference parameters. Removing std::move will cause compilation errors.

  void showInt(int&&) {}
  void testInt() {
  int a = 10;
  // expect: warning + nofix
  showInt(std::move(a));  // showInt(a) <--- wrong fix
  }
  
  struct Tmp {};
  void showTmp(Tmp&&) {}
  void testTmp() {
  Tmp t;
  // expect: warning + nofix
  showTmp(std::move(t));  // showTmp(t) <--- wrong fix
  }

case 2:
Using std::move() to wrap a pure rvalue or expiring value has no effect. Remove 
std::move().
The object returned in the function does not need to be wrapped with std::move. 
Because the returned nontrivially-copyable object will first call its own move 
constructor.

  struct TmpNR {
  TmpNR() {}
  TmpNR(const TmpNR&) {}
  TmpNR(TmpNR&&) {}
  };
  void showTmpNR(TmpNR&&) {}
  TmpNR testTmpNR() {
  TmpNR tnr;
  // expect: warning + fixit
  TmpNR tnr2 = std::move(TmpNR()); //  no warning <--- wrong diagnosis
  // expect: warning + fixit
  return std::move(tnr);  //  no warning <--- wrong diagnosis
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107450

Files:
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
@@ -70,7 +70,11 @@
   // CHECK-FIXES: return x3;
 }
 
-A f4(A x4) { return std::move(x4); }
+A f4(A x4) { 
+  return std::move(x4); 
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: passing result of std::move() as a const reference argument; no move will actually happen
+  // CHECK-FIXES: return x4;
+}
 
 A f5(const A x5) {
   return std::move(x5);
@@ -246,3 +250,73 @@
   };
   f(MoveSemantics());
 }
+
+void showInt(int &&) {}
+int testInt() {
+  int a = 10;
+  int b = std::move(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect; remove std::move()
+  // CHECK-FIXES: int b = a;
+  showInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect; remove std::move()
+  return std::move(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect; remove std::move()
+  // CHECK-FIXES: return a;
+}
+
+struct Tmp {};
+void showTmp(Tmp &&) {}
+Tmp testTmp() {
+  Tmp t;
+  Tmp t1 = std::move(t);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: std::move of the variable 't' of the trivially-copyable type 'Tmp' has no effect; remove std::move()
+  // CHECK-FIXES: Tmp t1 = t;
+  Tmp t2 = std::move(Tmp());
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: std::move of the expression of the trivially-copyable type 'Tmp' has no effect; remove std::move()
+  // CHECK-FIXES: Tmp t2 = Tmp();
+  showTmp(std::move(t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 't' of the trivially-copyable type 'Tmp' has no effect; remove std::move()
+  return std::move(t);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 't' of the trivially-copyable type 'Tmp' has no effect; remove std::move()
+  // CHECK-FIXES: return t;
+}
+
+struct Tmp_UnTriviallyC {
+  Tmp_UnTriviallyC() {}
+  Tmp_UnTriviallyC(const Tmp_UnTriviallyC &) {}
+};
+void showTmp_UnTriviallyC(Tmp_UnTriviallyC &&) {}
+Tmp_UnTriviallyC testTmp_UnTriviallyC() {
+  Tmp_UnTriviallyC tn;
+  Tmp_UnTriviallyC tn1 = std::move(tn);
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: passing result of std::move() as a const reference argument; no move will actually happen
+  // CHECK-FIXES: Tmp_UnTriviallyC tn1 = tn;
+  Tmp_UnTriviallyC tn2 = std::move(Tmp_UnTriviallyC());
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: passing result of std::move() as a const reference argument; no move will actually happen
+  // CHECK-FIXES: Tmp_UnTriviallyC tn2 = Tmp_UnTriviallyC();
+  showTmp_UnTriviallyC(std::move(tn));
+  // Expect no warning given here.
+  return std::move(tn);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: passing result of std::move() as a const reference argument; no move will actually happen 
+  // CHECK-FIXES: return tn;
+}
+
+struct Tmp_UnTriviallyCR {
+  Tmp_UnTriviallyCR() {}
+  Tmp_UnTriviallyCR(const Tmp_UnTriviall

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 364065.
aaron.ballman added a comment.
Herald added subscribers: kerbowa, nhaehnle, jvesely, emaste.

Fixing the tests I couldn't test locally but were found by the CI pipeline.


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

https://reviews.llvm.org/D107402

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/test/Driver/aarch64-outliner.c
  clang/test/Driver/aix-object-mode.c
  clang/test/Driver/amdgpu-invalid-target-id.s
  clang/test/Driver/amdgpu-openmp-system-arch-fail.c
  clang/test/Driver/arm-thumb-only-cores.c
  clang/test/Driver/cl-inputs.c
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/cuda-bad-arch.cu
  clang/test/Driver/cuda-detect.cu
  clang/test/Driver/cuda-omp-unsupported-debug-options.cu
  clang/test/Driver/cuda-options-freebsd.cu
  clang/test/Driver/cuda-options.cu
  clang/test/Driver/cuda-version-check.cu
  clang/test/Driver/defsym.s
  clang/test/Driver/fuse-ld.c
  clang/test/Driver/hip-inputs.hip
  clang/test/Driver/hip-invalid-target-id.hip
  clang/test/Driver/invalid-target-id.cl
  clang/test/Driver/msp430-hwmult.c
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/openmp-offload.c
  clang/test/Driver/rocm-detect.cl
  clang/test/Driver/rocm-detect.hip
  clang/test/Driver/rocm-not-found.cl
  clang/test/Frontend/invalid-cxx-abi.cpp
  clang/test/Frontend/round-trip-cc1-args.c
  clang/test/OpenMP/target_messages.cpp

Index: clang/test/OpenMP/target_messages.cpp
===
--- clang/test/OpenMP/target_messages.cpp
+++ clang/test/OpenMP/target_messages.cpp
@@ -8,12 +8,12 @@
 // CHECK: error: OpenMP target is invalid: 'aaa-bbb-ccc-ddd'
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx64-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
-// CHECK-UNSUPPORTED-HOST-TARGET: error: The target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target.
+// CHECK-UNSUPPORTED-HOST-TARGET: error: target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -fopenmp-targets=hexagon-linux-gnu -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-DEVICE-TARGET %s
 // CHECK-UNSUPPORTED-DEVICE-TARGET: OpenMP target is invalid: 'hexagon-linux-gnu'
 
 // RUN: not %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path .bc -o - 2>&1 | FileCheck --check-prefix NO-HOST-BC %s
-// NO-HOST-BC: The provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found.
+// NO-HOST-BC: provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found
 
 // RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm-bc %s -o %t-ppc-host.bc -DREGION_HOST
 // RUN: not %clang_cc1 -verify=expected,omp4 -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -DREGION_DEVICE 2>&1
Index: clang/test/Frontend/round-trip-cc1-args.c
===
--- clang/test/Frontend/round-trip-cc1-args.c
+++ clang/test/Frontend/round-trip-cc1-args.c
@@ -4,4 +4,4 @@
 
 // CHECK-WITHOUT-ROUND-TRIP-NOT: remark:
 // CHECK-ROUND-TRIP-WITHOUT-REMARKS-NOT: remark:
-// CHECK-ROUND-TRIP-WITH-REMARKS: remark: Generated arguments #{{.*}} in round-trip: {{.*}}
+// CHECK-ROUND-TRIP-WITH-REMARKS: remark: generated arguments #{{.*}} in round-trip: {{.*}}
Index: clang/test/Frontend/invalid-cxx-abi.cpp
===
--- clang/test/Frontend/invalid-cxx-abi.cpp
+++ clang/test/Frontend/invalid-cxx-abi.cpp
@@ -1,8 +1,8 @@
 // These shouldn't be valid -fc++-abi values.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=InvalidABI %s 2>&1 | FileCheck %s -check-prefix=INVALID
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=Fuchsia %s 2>&1 | FileCheck %s -check-prefix=CASE-SENSITIVE
-// INVALID: error: Invalid C++ ABI name 'InvalidABI'
-// CASE-SENSITIVE: error: Invalid C++ ABI name 'Fuchsia'
+// INVALID: error: invalid C++ ABI name 'InvalidABI'
+// CASE-SENSITIVE: error: invalid C++ ABI name 'Fuchsia'
 
 // Some C++ ABIs are not supported on some platforms.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=fuchsia -triple i386 %s 2>&1 | FileCheck %s -check-prefix=UNSUPPORTED-FUCHSIA
Index: clang/test/Driver/rocm-not-found.cl
=

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 364068.
ASDenysPetrov added a comment.

Corrected nits. Thanks to "the sharp eye" @vsavchenko =).


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

https://reviews.llvm.org/D107366

Files:
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp


Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -1289,15 +1289,32 @@
 return out;
 };
 
+var classListAdd = function(el, theClass) {
+  if (!el.className.baseVal)
+el.className += " " + theClass;
+  else
+el.className.baseVal += " " + theClass;
+};
+
+var classListRemove = function(el, theClass) {
+  var className = (!el.className.baseVal) ?
+  el.className : el.className.baseVal;
+className = className.replace(" " + theClass, "");
+  if (!el.className.baseVal)
+el.className = className;
+  else
+el.className.baseVal = className;
+};
+
 var scrollTo = function(el) {
 querySelectorAllArray(".selected").forEach(function(s) {
-s.classList.remove("selected");
+  classListRemove(s, "selected");
 });
-el.classList.add("selected");
+classListAdd(el, "selected");
 window.scrollBy(0, el.getBoundingClientRect().top -
 (window.innerHeight / 2));
 highlightArrowsForSelectedEvent();
-}
+};
 
 var move = function(num, up, numItems) {
   if (num == 1 && up || num == numItems - 1 && !up) {
@@ -1332,9 +1349,11 @@
   if (event.defaultPrevented) {
 return;
   }
-  if (event.key == "j") {
+  // key 'j'
+  if (event.keyCode == 74) {
 navigateTo(/*up=*/false);
-  } else if (event.key == "k") {
+  // key 'k'
+  } else if (event.keyCode == 75) {
 navigateTo(/*up=*/true);
   } else {
 return;
@@ -1350,8 +1369,11 @@
 

[PATCH] D106152: [analyzer] Move test case to existing test file and remove duplicated test file.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

I think the patch is pretty safe, then I decided to load it.


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

https://reviews.llvm.org/D106152

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

[PATCH] D106030: [Clang] add support for error+warning fn attrs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:3819
+  let Args = [StringArgument<"UserDiagnostic">];
+  let Subjects = SubjectList<[Function], ErrorDiag>;
+  let Documentation = [ErrorAttrDocs];

nickdesaulniers wrote:
> aaron.ballman wrote:
> > nickdesaulniers wrote:
> > > aaron.ballman wrote:
> > > > ObjC methods as well?
> > > I guess I can add them, but I'm unfamiliar with the language. If there's 
> > > no GNU implementation of an ObjC compiler, do we need to worry about GNU 
> > > C extensions in ObjC?
> > There's nothing platform-specific about the semantics of these attributes, 
> > so GNU or not doesn't really matter. I'm fine holding off on ObjC methods 
> > until a user requests support for it, but given how these are generally 
> > useful attributes, I think it would make sense to support ObjC up front 
> > unless there's a burden from supporting it.
> Looking at the generated IR for ObjC method calls, I don't think we can 
> support them (easily).
> 
> It looks like method calls in ObjC are lowered to direct calls to 
> `objc_msg_lookup`+indirect CALLs, IIUC.  This BackendDiagnostic relies on 
> observing direct calls during instruction selection. (I suspect ObjC supports 
> runtime modification of classes?) So during instruction selection, we can't 
> know that we're making a method call to a particular method (easily; maybe 
> there's helpers for this though?).  We don't seem to remove the indirection 
> even with optimizations enabled (making me think that methods can be swapped 
> at runtime).
> 
> We could try to diagnose in the frontend, but this very very quickly falls 
> apart if there's any control flow involved.  ie. maybe we could diagnose:
> ```
> void foo(void) { dontcall(); }
> ```
> but this whole BackendDiagnostic relies on the backend to run optimizations; 
> the frontend couldn't diagnose:
> ```
> void foo(void) { if (myRuntimeAssertion) dontcall(); }
> ```
> which is the predominate use case for this fn attr.
> 
> That said, the patch as is works for functions (not ObjC methods) in 
> objective-c mode already; would you like a test for that?
> 
> Marking as Done, please reopen this thread if I'm mistaken in this analysis.
> That said, the patch as is works for functions (not ObjC methods) in 
> objective-c mode already; would you like a test for that?

No thanks, I think the existing C coverage handles that.

> Marking as Done, please reopen this thread if I'm mistaken in this analysis.

Thanks for checking this out, I think it's a good reason to say "let's deal 
with this later (if ever)".



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:783
+  else
+Diags.Report(DiagID) << CalleeName << D.getMsgStr();
+}

nickdesaulniers wrote:
> aaron.ballman wrote:
> > nickdesaulniers wrote:
> > > nickdesaulniers wrote:
> > > > aaron.ballman wrote:
> > > > > nickdesaulniers wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > I think the usability in this situation is a concern -- only 
> > > > > > > having a function name but no source location information is 
> > > > > > > pretty tricky. Do you have a sense for how often we would hit 
> > > > > > > this case?
> > > > > > I don't think we ever encounter it when building the kernel 
> > > > > > sources. Perhaps I should make this an assertion on 
> > > > > > `LocCookie.isValid()` instead?
> > > > > > 
> > > > > > While clang should always attach a `srcloc` MetaData node, other 
> > > > > > frontend might not, so it's optional.  But this TU is strictly 
> > > > > > Clang.
> > > > > I thought backend optimization passes would drop source location 
> > > > > information with some regularity, so I'm a bit hesitant to go with an 
> > > > > assertion unless it turns out I'm wrong there. However, if the 
> > > > > backend shouldn't ever lose *this* source location information, then 
> > > > > I think it's good to make it an assertion.
> > > > Perhaps debug info, but this SourceLocation is squirreled away in a 
> > > > Metadata node attached to the call site.
> > > > 
> > > > Ah, but via further testing of @arsenm 's point ("what about indirect 
> > > > calls?"), this case is tricky:
> > > > ```
> > > > // clang -O2 foo.c
> > > > __attribute__((error("oh no foo")))
> > > > void foo(void);
> > > > 
> > > > void (*bar)(void);
> > > > 
> > > > void baz(void) {
> > > >   bar = foo;
> > > >   bar();
> > > > }
> > > > ```
> > > > since we did not emit the Metadata node on the call, but then later 
> > > > during optimizations we replaced the call to `bar` with a call to 
> > > > `foo`. At that point, the front end did not emit a Metadata node with 
> > > > source location information, so we can't reconstruct the call site 
> > > > properly for the diagnostic. Hmm...I'll need to think more about this 
> > > > case.
> > > One thing I was thinking of to support @arsenm's case:
> > > 
> > > We probably generally could support diagnosing indirect 

[PATCH] D105821: [analyzer] [WIP] Model destructor for std::unique_ptr

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 364074.
RedDocMD added a comment.

Better modelling, bug fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105821

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -753,10 +753,13 @@
 *Call, *this);
 
   ExplodedNodeSet DstInvalidated;
-  StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx);
-  for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
-   I != E; ++I)
-defaultEvalCall(Bldr, *I, *Call, CallOpts);
+  // StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx);
+  // for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E =
+  // DstPreCall.end();
+  //  I != E; ++I)
+  //   defaultEvalCall(Bldr, *I, *Call, CallOpts);
+  getCheckerManager().runCheckersForEvalCall(DstInvalidated, DstPreCall, *Call,
+ *this, CallOpts);
 
   getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated,
  *Call, *this);
Index: clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -664,14 +664,11 @@
 for (const auto &EvalCallChecker : EvalCallCheckers) {
   // TODO: Support the situation when the call doesn't correspond
   // to any Expr.
-  ProgramPoint L = ProgramPoint::getProgramPoint(
-  Call.getOriginExpr(), ProgramPoint::PostStmtKind,
-  Pred->getLocationContext(), EvalCallChecker.Checker);
   bool evaluated = false;
   { // CheckerContext generates transitions(populates checkDest) on
 // destruction, so introduce the scope to make sure it gets properly
 // populated.
-CheckerContext C(B, Eng, Pred, L);
+CheckerContext C(B, Eng, Pred, Call.getProgramPoint());
 evaluated = EvalCallChecker(Call, C);
   }
   assert(!(evaluated && anyEvaluated)
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -139,7 +139,7 @@
 
   if (RD->getDeclName().isIdentifier()) {
 StringRef Name = RD->getName();
-return Name == "shared_ptr" || Name == "unique_ptr" || Name == "weak_ptr";
+return llvm::is_contained(STD_PTR_NAMES, Name);
   }
   return false;
 }
@@ -275,6 +275,29 @@
  smartptr::isStdSmartPtr(Call.getArgExpr(1));
 }
 
+std::pair
+invalidateInnerPointer(const MemRegion *ThisRegion, ProgramStateRef State,
+   const CallEvent &Call, CheckerContext &C) {
+  const auto *InnerPtrVal = State->get(ThisRegion);
+  if (InnerPtrVal) {
+State = State->invalidateRegions(*InnerPtrVal, nullptr, C.blockCount(),
+ C.getLocationContext(), true);
+
+const QualType &Type = getInnerPointerType(Call, C);
+const auto *RD = Type->getAsCXXRecordDecl();
+if (!RD)
+  return {State, false};
+const auto *DD = RD->getDestructor();
+
+const auto InnerDestrCall =
+C.getStateManager().getCallEventManager().getCXXDestructorCall(
+DD, nullptr, InnerPtrVal->getAsRegion(), RD->bases().empty(), State,
+C.getLocationContext());
+InnerDestrCall->invalidateRegions(C.blockCount(), State);
+  }
+  return {State, true};
+}
+
 bool SmartPtrModeling::evalCall(const CallEvent &Call,
 CheckerContext &C) const {
 
@@ -372,6 +395,25 @@
 }
   }
 
+  if (const auto *DC = dyn_cast(&Call)) {
+const MemRegion *ThisRegion = DC->getCXXThisVal().getAsRegion();
+if (!ThisRegion)
+  return false;
+State = State->remove(ThisRegion);
+bool ShouldGiveUp;
+std::tie(State, ShouldGiveUp) =
+invalidateInnerPointer(ThisRegion, State, Call, C);
+if (ShouldGiveUp)
+  return false;
+// This tag is required to prevent later crashes due to the non-addition
+// of new States. Having a tag ensures that the call to addTransition
+// actually adds a new state.
+static SimpleProgramPointTag SPPT("SmartPtrModeling",
+  "on destructor modeling");
+C.addTransition(State, &SPPT);
+return true;
+  }
+
   if (!ModelSmartPtrDereference)
 return false;
 
@@ -402,8 +444,8 @@
   }));
 } else {
   const auto *TrackingExpr = 

[PATCH] D107294: [clang-tidy] adds warning to suggest users replace symbols with words

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp:133-134
 "readability-uppercase-literal-suffix");
+CheckFactories.registerCheck(
+"readability-use-alternative-tokens");
 CheckFactories.registerCheck(

I think this might be a case where we want the check to either recommend using 
alternative tokens or recommend against using alternative tokens via a 
configuration option (so users can control readability in either direction). If 
you agree that's a reasonable design, then I'd recommend we name this 
`readability-alternative-tokens` to be a bit more generic. (Note, we can always 
do the "don't use alternative tokens" implementation in a follow-up patch if 
you don't want to do that work immediately.)



Comment at: 
clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.cpp:53-54
+  bool IsLogicalNot = Op.getOpcode() == UnaryOperator::Opcode::UO_LNot;
+  auto Hint = FixItHint::CreateReplacement(Op.getOperatorLoc(),
+   IsLogicalNot ? "not " : "compl ");
+

For C code, you could augment this FixIt with an include inserter, e.g., 
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp#L112



Comment at: 
clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.h:25
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+return LangOpts.CPlusPlus;
+  }

We can support C for this as well, can't we? iso646.h has been around since C99.



Comment at: 
clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.h:5-6
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

cjdb wrote:
> whisperity wrote:
> > This seems to be very old code, there was a licence change approx. 2 years 
> > ago.
> Yikes, that's what I get for not reading what I'm copying. Should there be an 
> NFC to go and update the other checks' licences too?
> Should there be an NFC to go and update the other checks' licences too?

I think that'd be useful!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107294

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

[PATCH] D107420: [WIP][sema] Disallow __builtin_mul_overflow under special condition.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a subscriber: aaron.ballman.
erichkeane added a comment.

This seems lie an incomplete fix.  Is this the ONLY configuration that this has 
trouble with?

Also, the error message seems awkward, @aaron.ballman : Can you suggest a 
better spelling?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107420

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

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

This discussion is becoming pointless. Clang is not a 1:1 replacement for later 
GCC versions. Easiest example is the support for `__builtin_apply` and friends.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107304

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

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 364078.
ASDenysPetrov added a comment.

Replaced a closure in `highlightArrowsForSelectedEvent ` with function object.


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

https://reviews.llvm.org/D107366

Files:
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp


Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -1289,15 +1289,32 @@
 return out;
 };
 
+var classListAdd = function(el, theClass) {
+  if(!el.className.baseVal)
+el.className += " " + theClass;
+  else
+el.className.baseVal += " " + theClass;
+};
+
+var classListRemove = function(el, theClass) {
+  var className = (!el.className.baseVal) ?
+  el.className : el.className.baseVal;
+className = className.replace(" " + theClass, "");
+  if(!el.className.baseVal)
+el.className = className;
+  else
+el.className.baseVal = className;
+};
+
 var scrollTo = function(el) {
 querySelectorAllArray(".selected").forEach(function(s) {
-s.classList.remove("selected");
+  classListRemove(s, "selected");
 });
-el.classList.add("selected");
+classListAdd(el, "selected");
 window.scrollBy(0, el.getBoundingClientRect().top -
 (window.innerHeight / 2));
 highlightArrowsForSelectedEvent();
-}
+};
 
 var move = function(num, up, numItems) {
   if (num == 1 && up || num == numItems - 1 && !up) {
@@ -1332,9 +1349,11 @@
   if (event.defaultPrevented) {
 return;
   }
-  if (event.key == "j") {
+  // key 'j'
+  if (event.keyCode == 74) {
 navigateTo(/*up=*/false);
-  } else if (event.key == "k") {
+  // key 'k'
+  } else if (event.keyCode == 75) {
 navigateTo(/*up=*/true);
   } else {
 return;
@@ -1350,8 +1369,11 @@
 

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

F18343863: report-cdf7f5.html  This is a 
sample file which now should work both on IE and Chromium-based browsers.


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

https://reviews.llvm.org/D107366

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

[PATCH] D105821: [analyzer] [WIP] Model destructor for std::unique_ptr

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

I have incorporated the bug-fixes suggested last meeting (except the pointer 
escape one). And it seems to have had dramatic results - now the only extra 
errors being reported are the pointer escape ones (5 of them, from 3 different 
projects). Some projects are actually reporting that bug reports have been 
//removed// due to this patch.F18343949: Error List 2.pdf 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105821

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

[PATCH] D107420: [WIP][sema] Disallow __builtin_mul_overflow under special condition.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:334-335
+const auto ResultTy = TheCall->getArg(2)->getType()->getPointeeType();
+// Input compination below will also emit integer value larger than
+// 128 bits in backend, disallow same as above.
+if (LeftTy->isSignedIntegerType() &&





Comment at: clang/lib/Sema/SemaChecking.cpp:340
+S.getASTContext().getIntWidth(RightTy) > 64 &&
+!ResultTy->isSignedIntegerType() &&
+S.getASTContext().getIntWidth(ResultTy) > 64) {

Isn't the result type a pointer type?



Comment at: clang/test/Sema/builtins-overflow.c:45
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, &result); // expected-error 
{{__builtin_mul_overflow does not support special combination operands (signed, 
signed, unsigned*) of more than 64 bits}}
+  }

Yeah, this diagnostic really doesn't tell me what's going wrong with the code 
or how to fix it. Do we basically want to prevent using larger-than-64-bit 
argument types with mixed signs? Or are there other problematic circumstances?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107420

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

[PATCH] D107430: [OMPIRBuilder] Add ordered without depend and simd clause to OMPBuilder

[Peixin Qiao via Phabricator via cfe-commits]

peixin added a comment.

@kiranchandramohan Thanks for the review. Removed the OpenMP 1.0 target from 
the description.

The simd clause does need more changes and I think it also depend simd 
directive in the OpenMPIRBuilder. According to the CHECK5 in the test case 
`ordered_codegen.cpp`, it does not create `kmp_ordered` runtime functions call 
with the simd clause.

I have not investigated too much about the standalone ordered construct. To be 
honest, I am not sure if supporting simd and depend clauses in CreateOrdered is 
better or not. My current plan is to look into the Flang MLIR Op Def and LLVM 
IR to understand how the IRBuilder is used. After finishing the end-to-end 
implementation of ordered construct without simd and depend clauses, I will 
come back to try to implement the IRBuilder of ordered directive with simd and 
depend clauses. At that time, Arnamoy should also have time to implement the 
IRBuilder for simd directive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107430

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

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Just some thoughts!  See what you like/don't like here, i'm not attached to my 
suggestions.




Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:86
+def err_drv_mix_cuda_hip : Error<
+  "mixed CUDA and HIP compilation is not supported">;
+def err_drv_bad_target_id : Error<

I wonder if this should be something more like "invalid argument '' not allowed in 'CUDA'".

Would be more consistent with the options mixed with C/C++ mode/etc.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:88
+def err_drv_bad_target_id : Error<
+  "invalid target ID: %0 (a target ID is a processor name followed by an "
+  "optional list of predefined features post-fixed by a plus or minus sign "

  invalid target ID %0; format is processor name followed by an optional colon 
delimited list of features followed by enable/disable sign, .e.g. 
'gfx908:sramecc+;xnack-'

??  I think we should be leaning on the example more to explain the format.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:92
+def err_drv_bad_offload_arch_combo : Error<
+  "invalid offload arch combinations: %0 and %1 (for a specific processor, a "
+  "feature should either exist in all offload archs, or not exist in any "

This is an improvement, but this one is... rough.  Not sure enough of what it 
is saying unfortunately to suggest a better wording.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:276
 def err_drv_omp_host_ir_file_not_found : Error<
-  "The provided host compiler IR file '%0' is required to generate code for 
OpenMP target regions but cannot be found.">;
+  "provided host compiler IR file '%0' is required to generate code for OpenMP 
"
+  "target regions but cannot be found">;

provided host compiler IR file '%0' not found; required to generate code 
for OpenMP target regions

??



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:319
 def warn_drv_dwarf_version_limited_by_target : Warning<
-  "debug information option '%0' is not supported. It needs DWARF-%2 but 
target '%1' only provides DWARF-%3.">,
+  "debug information option '%0' is not supported; it needs DWARF-%2 but "
+  "target '%1' only provides DWARF-%3">,

it 'requires' perhaps?



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:524
 def warn_drv_moutline_unsupported_opt : Warning<
-  "The '%0' architecture does not support -moutline; flag ignored">,
+  "the '%0' architecture does not support '-moutline'; flag ignored">,
   InGroup;

consider just removing 'the' from this and the next one.


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

https://reviews.llvm.org/D107402

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

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a subscriber: kbsmith1.
mibintc added a comment.

There was a long discussion on cfe-dev about this issue approximately 
January-February 2020 including @scanon @andrew.w.kaylor and many others.  
While there wasn't 100% consensus to move to ffp-contract=on there are many 
reasons to do it including better numerical results and debuggability.  I'm 
hoping that @andrew.w.kaylor @kbsmith1 @rjmccall and others will join in here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74436

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

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:88
+def err_drv_bad_target_id : Error<
+  "invalid target ID: %0 (a target ID is a processor name followed by an "
+  "optional list of predefined features post-fixed by a plus or minus sign "

erichkeane wrote:
>   invalid target ID %0; format is processor name followed by an optional 
> colon delimited list of features followed by enable/disable sign, .e.g. 
> 'gfx908:sramecc+;xnack-'
> 
> ??  I think we should be leaning on the example more to explain the format.
I think that's an improvement; the current wording is... hard to interpret. I 
tweaked it slightly, WDYT?



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:92
+def err_drv_bad_offload_arch_combo : Error<
+  "invalid offload arch combinations: %0 and %1 (for a specific processor, a "
+  "feature should either exist in all offload archs, or not exist in any "

erichkeane wrote:
> This is an improvement, but this one is... rough.  Not sure enough of what it 
> is saying unfortunately to suggest a better wording.
Yeah, I couldn't think of a better way to phrase it, so I figured the original 
wording is sufficient with some minor cleanups. I'll probably leave this one 
alone unless someone has a great idea.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:276
 def err_drv_omp_host_ir_file_not_found : Error<
-  "The provided host compiler IR file '%0' is required to generate code for 
OpenMP target regions but cannot be found.">;
+  "provided host compiler IR file '%0' is required to generate code for OpenMP 
"
+  "target regions but cannot be found">;

erichkeane wrote:
> provided host compiler IR file '%0' not found; required to generate code 
> for OpenMP target regions
> 
> ??
I think the original wording reads somewhat better because it is grammatically 
correct. I don't think we gain a lot by replacing `is` with a semicolon here.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:319
 def warn_drv_dwarf_version_limited_by_target : Warning<
-  "debug information option '%0' is not supported. It needs DWARF-%2 but 
target '%1' only provides DWARF-%3.">,
+  "debug information option '%0' is not supported; it needs DWARF-%2 but "
+  "target '%1' only provides DWARF-%3">,

erichkeane wrote:
> it 'requires' perhaps?
Good call!



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:524
 def warn_drv_moutline_unsupported_opt : Warning<
-  "The '%0' architecture does not support -moutline; flag ignored">,
+  "the '%0' architecture does not support '-moutline'; flag ignored">,
   InGroup;

erichkeane wrote:
> consider just removing 'the' from this and the next one.
Good call, but I'm also going to drop `architecture` from it so it's just `'%0' 
does not support...`


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

https://reviews.llvm.org/D107402

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

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 364093.
aaron.ballman marked 5 inline comments as done.
aaron.ballman added a comment.

Updating based on review feedback and restarting the CI pipeline.


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

https://reviews.llvm.org/D107402

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/test/Driver/aarch64-outliner.c
  clang/test/Driver/aix-object-mode.c
  clang/test/Driver/amdgpu-invalid-target-id.s
  clang/test/Driver/amdgpu-openmp-system-arch-fail.c
  clang/test/Driver/arm-thumb-only-cores.c
  clang/test/Driver/cl-inputs.c
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/cuda-bad-arch.cu
  clang/test/Driver/cuda-detect.cu
  clang/test/Driver/cuda-omp-unsupported-debug-options.cu
  clang/test/Driver/cuda-options-freebsd.cu
  clang/test/Driver/cuda-options.cu
  clang/test/Driver/cuda-version-check.cu
  clang/test/Driver/defsym.s
  clang/test/Driver/fuse-ld.c
  clang/test/Driver/hip-inputs.hip
  clang/test/Driver/hip-invalid-target-id.hip
  clang/test/Driver/invalid-target-id.cl
  clang/test/Driver/msp430-hwmult.c
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/openmp-offload.c
  clang/test/Driver/rocm-detect.cl
  clang/test/Driver/rocm-detect.hip
  clang/test/Driver/rocm-not-found.cl
  clang/test/Frontend/invalid-cxx-abi.cpp
  clang/test/Frontend/round-trip-cc1-args.c
  clang/test/OpenMP/target_messages.cpp

Index: clang/test/OpenMP/target_messages.cpp
===
--- clang/test/OpenMP/target_messages.cpp
+++ clang/test/OpenMP/target_messages.cpp
@@ -8,12 +8,12 @@
 // CHECK: error: OpenMP target is invalid: 'aaa-bbb-ccc-ddd'
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx64-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
-// CHECK-UNSUPPORTED-HOST-TARGET: error: The target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target.
+// CHECK-UNSUPPORTED-HOST-TARGET: error: target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -fopenmp-targets=hexagon-linux-gnu -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-DEVICE-TARGET %s
 // CHECK-UNSUPPORTED-DEVICE-TARGET: OpenMP target is invalid: 'hexagon-linux-gnu'
 
 // RUN: not %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path .bc -o - 2>&1 | FileCheck --check-prefix NO-HOST-BC %s
-// NO-HOST-BC: The provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found.
+// NO-HOST-BC: provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found
 
 // RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm-bc %s -o %t-ppc-host.bc -DREGION_HOST
 // RUN: not %clang_cc1 -verify=expected,omp4 -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -DREGION_DEVICE 2>&1
Index: clang/test/Frontend/round-trip-cc1-args.c
===
--- clang/test/Frontend/round-trip-cc1-args.c
+++ clang/test/Frontend/round-trip-cc1-args.c
@@ -4,4 +4,4 @@
 
 // CHECK-WITHOUT-ROUND-TRIP-NOT: remark:
 // CHECK-ROUND-TRIP-WITHOUT-REMARKS-NOT: remark:
-// CHECK-ROUND-TRIP-WITH-REMARKS: remark: Generated arguments #{{.*}} in round-trip: {{.*}}
+// CHECK-ROUND-TRIP-WITH-REMARKS: remark: generated arguments #{{.*}} in round-trip: {{.*}}
Index: clang/test/Frontend/invalid-cxx-abi.cpp
===
--- clang/test/Frontend/invalid-cxx-abi.cpp
+++ clang/test/Frontend/invalid-cxx-abi.cpp
@@ -1,8 +1,8 @@
 // These shouldn't be valid -fc++-abi values.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=InvalidABI %s 2>&1 | FileCheck %s -check-prefix=INVALID
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=Fuchsia %s 2>&1 | FileCheck %s -check-prefix=CASE-SENSITIVE
-// INVALID: error: Invalid C++ ABI name 'InvalidABI'
-// CASE-SENSITIVE: error: Invalid C++ ABI name 'Fuchsia'
+// INVALID: error: invalid C++ ABI name 'InvalidABI'
+// CASE-SENSITIVE: error: invalid C++ ABI name 'Fuchsia'
 
 // Some C++ ABIs are not supported on some platforms.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=fuchsia -triple i386 %s 2>&1 | FileCheck %s -check-prefix=UNSUPPORTED-FUCHSIA
Index: clang/test/Driver/rocm-not-found.cl
=

[PATCH] D105263: [X86] AVX512FP16 instructions enabling 1/6

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:3405
+/// half member at the specified offset.  For example, {int,{half}} has a
+/// float at offset 4.  It is conservatively correct for this routine to return
+/// false.

float -> half?



Comment at: clang/lib/Headers/avx512fp16intrin.h:292
+
+  return (__m128h)__builtin_ia32_loadsh128_mask((__v8hf *)__A, src, __U & 1);
+}

Just be curious, why not directly use __W?



Comment at: clang/lib/Headers/avx512fp16intrin.h:319
+__m512h_u __v;
+  } __attribute__((__packed__, __may_alias__));
+  return ((const struct __loadu_ph *)__p)->__v;

What is __may_alias__ used for?



Comment at: clang/lib/Headers/avx512fp16intrin.h:350
+   __m128h __A) {
+  __builtin_ia32_storesh128_mask((__v8hf *)__W, __A, __U & 1);
+}

I see in _mm_mask_load_sh(), we create a __m128h with upper bits zero, not sure 
we also need it in store intrinsic.



Comment at: clang/lib/Headers/avx512fp16intrin.h:419
+static __inline__ short __DEFAULT_FN_ATTRS128 _mm_cvtsi128_si16(__m128i __a) {
+  __v8hi __b = (__v8hi)__a;
+  return __b[0];

Why not return __a[0] directly?



Comment at: clang/test/CodeGen/X86/avx512fp16-abi.c:89
+  _Float16 a;
+  float b;
+};

Any false test case that have padding between a and b?



Comment at: llvm/include/llvm/IR/Intrinsics.td:315
 def llvm_v8f16_ty  : LLVMType;//  8 x half (__fp16)
+def llvm_v16f16_ty : LLVMType;   // 16 x half (__fp16)
+def llvm_v32f16_ty : LLVMType;   // 32 x half (__fp16)

Not sure about the legacy comments, should it be _Float16 now?



Comment at: llvm/include/llvm/Target/TargetSelectionDAG.td:1054
+def extloadvf16 : PatFrag<(ops node:$ptr), (extload node:$ptr)> {
+  let IsLoad = 1;
+  let ScalarMemoryVT = f16;

I notice it is true for other extload. Is it same to "true"?



Comment at: llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp:341
 if ((insn->mode == MODE_64BIT || (byte1 & 0xc0) == 0xc0) &&
-((~byte1 & 0xc) == 0xc) && ((byte2 & 0x4) == 0x4)) {
+((~byte1 & 0x8) == 0x8) && ((byte2 & 0x4) == 0x4)) {
   insn->vectorExtensionType = TYPE_EVEX;

This is the same to ((byte1 & 0x8) == 0x0)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105263

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

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D107402#2925402 , @erichkeane 
wrote:

> The changes in this patch are all fine to me! I'm a little disturbed that it 
> only broke 1 test though...

Thanks for the review!

The test coverage for driver diagnostics definitely appears to be lacking. I 
don't intend to add the missing test coverage myself as part of this cleanup, 
but when we review driver changes, we should be more insistent at requiring 
tests to exercise the diagnostics.


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

https://reviews.llvm.org/D107402

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

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

The changes in this patch are all fine to me! I'm a little disturbed that it 
only broke 1 test though...


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

https://reviews.llvm.org/D107402

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

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: 
clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h:38
+
+#pragma omp begin declare variant match(device = {arch(amdgcn)})
+

Given that declare variant didn't work elsewhere, it probably doesn't work 
here. Thus this may be the root cause of 
https://bugs.llvm.org/show_bug.cgi?id=51337


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104904

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

[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

[Sean Fertile via Phabricator via cfe-commits]

sfertile marked 2 inline comments as done.
sfertile added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1783
+if (FieldPacked) {
+  FieldAlign = UnpackedFieldAlign = 1;
+} else if (!MaxFieldAlignment.isZero()) {

stevewan wrote:
> `UnpackedFieldAlign` is used to check if the packed attribute is unnecessary 
> (-Wpacked). here the attribute is making a difference, we probably shouldn't 
> set `FieldAlign = UnpackedFieldAlign`?
Good catch, updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106900

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

[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

[Sean Fertile via Phabricator via cfe-commits]

sfertile updated this revision to Diff 364097.
sfertile added a comment.

Don't update the unpacked field align based on IsPacked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106900

Files:
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/Layout/aix-packed-bitfields.c


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
 
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
 
 struct A {
   int a1 : 30;
@@ -75,3 +79,35 @@
 // CHECK-NEXT: 3:6-35 |   int a2
 // CHECK-NEXT:  7:4-7 |   int a3
 // CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
+//
+struct __attribute__((packed)) PackedAttr {
+  char f1;
+  int : 0;
+  short : 3;
+  char f4 : 2;
+};
+
+int e = sizeof(struct PackedAttr);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttr
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4:0-2 |   short
+// CHECK-NEXT:  4:3-4 |   char f4
+// CHECK-NEXT:  sizeof=5, {{(dsize=5, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct __attribute__((packed)) PackedAttrAndPragma {
+  char f1;
+  long long : 0;
+};
+#pragma pack(pop)
+
+int f = sizeof(struct PackedAttrAndPragma);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttrAndPragma
+// CHECK-NEXT:  0 |   char f1
+// 32BIT-NEXT:4:- |   long long
+// 32BIT-NEXT:  sizeof=4, {{(dsize=4, )?}}align=1, preferredalign=1
+// 64BIT-NEXT:8:- |   long long
+// 64BIT-NEXT:  sizeof=8, {{(dsize=8, )?}}align=1, preferredalign=1
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1775,11 +1775,18 @@
   !D->getIdentifier())
 FieldAlign = UnpackedFieldAlign = 1;
 
-  // On AIX, zero-width bitfields pad out to the alignment boundary, but then
-  // do not affect overall record alignment if there is a pragma pack or
-  // pragma align(packed).
-  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
-FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  // On AIX, zero-width bitfields pad out to the natural alignment boundary,
+  // but do not increase the alignment greater than the MaxFieldAlignment, or 1
+  // if packed.
+  if (isAIXLayout(Context) && !FieldSize) {
+if (FieldPacked)
+  FieldAlign = 1;
+if (!MaxFieldAlignment.isZero()) {
+  UnpackedFieldAlign =
+  std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits);
+  FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+}
+  }
 
   // Diagnose differences in layout due to padding or packing.
   if (!UseExternalLayout)


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
 
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
+
 // RUN: %clang_cc1 -tripl

[PATCH] D107461: [PowerPC] Do not define __PRIVILEGED__

[Stefan Pintilie via Phabricator via cfe-commits]

stefanp created this revision.
stefanp added reviewers: nemanjai, lei.
Herald added subscribers: shchenz, kbarton.
stefanp requested review of this revision.
Herald added a project: clang.

We do not want to define __PRIVILEGED__. There is no use case for the
definition and gcc does not define it. This patch removes that definition.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107461

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/Driver/ppc-mprivileged-support-check.c
  clang/test/Preprocessor/init-ppc64.c

Index: clang/test/Preprocessor/init-ppc64.c
===
--- clang/test/Preprocessor/init-ppc64.c
+++ clang/test/Preprocessor/init-ppc64.c
@@ -565,7 +565,6 @@
 // PPCPWR8:#define _ARCH_PWR7 1
 // PPCPWR8:#define _ARCH_PWR8 1
 // PPCPWR8-NOT:#define __ROP_PROTECT__ 1
-// PPCPWR8-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu power8 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCPOWER8 %s
 //
@@ -584,7 +583,6 @@
 // PPCPOWER8:#define _ARCH_PWR7 1
 // PPCPOWER8:#define _ARCH_PWR8 1
 // PPCPOWER8-NOT:#define __ROP_PROTECT__ 1
-// PPCPOWER8-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu pwr9 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCPWR9 %s
 //
@@ -600,7 +598,6 @@
 // PPCPWR9:#define _ARCH_PWR7 1
 // PPCPWR9:#define _ARCH_PWR9 1
 // PPCPWR9-NOT:#define __ROP_PROTECT__ 1
-// PPCPWR9-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu power9 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCPOWER9 %s
 //
@@ -616,7 +613,6 @@
 // PPCPOWER9:#define _ARCH_PWR7 1
 // PPCPOWER9:#define _ARCH_PWR9 1
 // PPCPOWER9-NOT:#define __ROP_PROTECT__ 1
-// PPCPOWER9-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu pwr10 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCPOWER10 %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu power10 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCPOWER10 %s
@@ -637,7 +633,6 @@
 // PPCPOWER10:#define __MMA__ 1
 // PPCPOWER10:#define __PCREL__ 1
 // PPCPOWER10-NOT:#define __ROP_PROTECT__ 1
-// PPCPOWER10-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu future -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCFUTURE %s
 //
@@ -658,7 +653,6 @@
 // PPCFUTURE:#define __MMA__ 1
 // PPCFUTURE:#define __PCREL__ 1
 // PPCFUTURE-NOT:#define __ROP_PROTECT__ 1
-// PPCFUTURE-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +mma -target-cpu power10 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-MMA %s
 // PPC-MMA:#define __MMA__ 1
@@ -668,11 +662,6 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +rop-protect -target-cpu power8 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-ROP %s
 // PPC-ROP:#define __ROP_PROTECT__ 1
 //
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +privileged -target-cpu power10 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-PRIV %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +privileged -target-cpu power9 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-PRIV %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +privileged -target-cpu power8 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-PRIV %s
-// PPC-PRIV:#define __PRIVILEGED__ 1
-//
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +float128 -target-cpu power9 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-FLOAT128 %s
 // PPC-FLOAT128:#define __FLOAT128__ 1
 //
Index: clang/test/Driver/ppc-mprivileged-support-check.c
===
--- clang/test/Driver/ppc-mprivileged-support-check.c
+++ clang/test/Driver/ppc-mprivileged-support-check.c
@@ -1,26 +1,29 @@
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr10 -mprivileged %s 2>&1 | FileCheck %s --check-prefix=HASPRIV
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=power10 -mprivileged %s 2>&1 | FileCheck %s --check-prefix=HASPRIV
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr9 -mprivileged %s 2>&1 | FileCheck %s --check-prefix=HASPRIV
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=power9 -mprivileged %s 2>&1 | FileCheck %s

[PATCH] D107244: [AIX] Define _ARCH_PPC64 macro for 32-bit

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm requested changes to this revision.
cebowleratibm added a comment.
This revision now requires changes to proceed.

The divergence with GCC is regrettable.  Unfortunately defining _ARCH_PPC64 in 
32-bit has been a long-standing discrepancy between xlc and gcc and one I don't 
expect gcc to change.  This patch is preferable in order to retain 
preprocessing behaviour on AIX for customers porting 32-bit AIX xlc 
applications to clang.

My request for change is simply to add a brief explanation in the code, 
otherwise, it looks like a coding error.




Comment at: clang/lib/Basic/Targets/PPC.cpp:260
+  } else {
+if (getTriple().isOSAIX())
+  Builder.defineMacro("_ARCH_PPC64");

A comment is warranted here to explain that __ARCH_PPC64 is only defined in 
32-bit on AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107244

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

[PATCH] D105690: [RISCV] Rename assembler mnemonic of unordered floating-point reductions for v1.0-rc change

[Fraser Cormack via Phabricator via cfe-commits]

frasercrmck added a comment.

Sorry, I can't find if we wrote it down in some other patch -- and someone can 
correct me if I'm wrong -- but in one of the recent LLVM RISC-V sync-up calls 
we agreed that we'd skip v0.10-rc and move straight to supporting v1.0 when 
it's made final. So I think this patch will probably have to wait for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105690

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

[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

[Steven Wan via Phabricator via cfe-commits]

stevewan accepted this revision.
stevewan added a comment.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106900

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

[clang] b8f612e - [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

[Sean Fertile via cfe-commits]

Author: Sean Fertile
Date: 2021-08-04T11:03:25-04:00
New Revision: b8f612e780e50cfb62bc0196b6367e4587949f88

URL: 
https://github.com/llvm/llvm-project/commit/b8f612e780e50cfb62bc0196b6367e4587949f88
DIFF: 
https://github.com/llvm/llvm-project/commit/b8f612e780e50cfb62bc0196b6367e4587949f88.diff

LOG: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

Zero-width bitfields on AIX pad out to the natral alignment boundary but
do not change the containing records alignment.

Differential Revision: https://reviews.llvm.org/D106900

Added: 


Modified: 
clang/lib/AST/RecordLayoutBuilder.cpp
clang/test/Layout/aix-packed-bitfields.c

Removed: 




diff  --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 972690becf9ec..83045253aa512 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1775,11 +1775,18 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   !D->getIdentifier())
 FieldAlign = UnpackedFieldAlign = 1;
 
-  // On AIX, zero-width bitfields pad out to the alignment boundary, but then
-  // do not affect overall record alignment if there is a pragma pack or
-  // pragma align(packed).
-  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
-FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  // On AIX, zero-width bitfields pad out to the natural alignment boundary,
+  // but do not increase the alignment greater than the MaxFieldAlignment, or 1
+  // if packed.
+  if (isAIXLayout(Context) && !FieldSize) {
+if (FieldPacked)
+  FieldAlign = 1;
+if (!MaxFieldAlignment.isZero()) {
+  UnpackedFieldAlign =
+  std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits);
+  FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+}
+  }
 
   // Diagnose 
diff erences in layout due to padding or packing.
   if (!UseExternalLayout)

diff  --git a/clang/test/Layout/aix-packed-bitfields.c 
b/clang/test/Layout/aix-packed-bitfields.c
index 9bc907af0f596..88f6b3fced80a 100644
--- a/clang/test/Layout/aix-packed-bitfields.c
+++ b/clang/test/Layout/aix-packed-bitfields.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
 
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
 
 struct A {
   int a1 : 30;
@@ -75,3 +79,35 @@ int d = sizeof(struct Pack2);
 // CHECK-NEXT: 3:6-35 |   int a2
 // CHECK-NEXT:  7:4-7 |   int a3
 // CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
+//
+struct __attribute__((packed)) PackedAttr {
+  char f1;
+  int : 0;
+  short : 3;
+  char f4 : 2;
+};
+
+int e = sizeof(struct PackedAttr);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttr
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4:0-2 |   short
+// CHECK-NEXT:  4:3-4 |   char f4
+// CHECK-NEXT:  sizeof=5, {{(dsize=5, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct __attribute__((packed)) PackedAttrAndPragma {
+  char f1;
+  long long : 0;
+};
+#pragma pack(pop)
+
+int f = sizeof(struct PackedAttrAndPragma);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttrAndPragma
+// CHECK-NEXT:  0 |   char f1
+// 32BIT-NEXT:4:- |   long long
+// 32BIT-NEXT:  sizeof=4, {{(dsize=4, )?}}align=1, preferredalign=1
+// 64BIT-NEXT:8:- |   long long
+// 64BIT-NEXT:  sizeof=8, {{(dsize=8, )?}}align=1, preferredalign=1



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

[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

[Sean Fertile via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sfertile marked an inline comment as done.
Closed by commit rGb8f612e780e5: [PowerPC][AIX] Packed zero-width bitfields do 
not affect alignment. (authored by sfertile).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106900

Files:
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/Layout/aix-packed-bitfields.c


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
 
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
 
 struct A {
   int a1 : 30;
@@ -75,3 +79,35 @@
 // CHECK-NEXT: 3:6-35 |   int a2
 // CHECK-NEXT:  7:4-7 |   int a3
 // CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
+//
+struct __attribute__((packed)) PackedAttr {
+  char f1;
+  int : 0;
+  short : 3;
+  char f4 : 2;
+};
+
+int e = sizeof(struct PackedAttr);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttr
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4:0-2 |   short
+// CHECK-NEXT:  4:3-4 |   char f4
+// CHECK-NEXT:  sizeof=5, {{(dsize=5, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct __attribute__((packed)) PackedAttrAndPragma {
+  char f1;
+  long long : 0;
+};
+#pragma pack(pop)
+
+int f = sizeof(struct PackedAttrAndPragma);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttrAndPragma
+// CHECK-NEXT:  0 |   char f1
+// 32BIT-NEXT:4:- |   long long
+// 32BIT-NEXT:  sizeof=4, {{(dsize=4, )?}}align=1, preferredalign=1
+// 64BIT-NEXT:8:- |   long long
+// 64BIT-NEXT:  sizeof=8, {{(dsize=8, )?}}align=1, preferredalign=1
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1775,11 +1775,18 @@
   !D->getIdentifier())
 FieldAlign = UnpackedFieldAlign = 1;
 
-  // On AIX, zero-width bitfields pad out to the alignment boundary, but then
-  // do not affect overall record alignment if there is a pragma pack or
-  // pragma align(packed).
-  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
-FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  // On AIX, zero-width bitfields pad out to the natural alignment boundary,
+  // but do not increase the alignment greater than the MaxFieldAlignment, or 1
+  // if packed.
+  if (isAIXLayout(Context) && !FieldSize) {
+if (FieldPacked)
+  FieldAlign = 1;
+if (!MaxFieldAlignment.isZero()) {
+  UnpackedFieldAlign =
+  std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits);
+  FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+}
+  }
 
   // Diagnose differences in layout due to padding or packing.
   if (!UseExternalLayout)


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
 
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pac

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

[Pushpinder Singh via Phabricator via cfe-commits]

pdhaliwal added inline comments.



Comment at: 
clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h:38
+
+#pragma omp begin declare variant match(device = {arch(amdgcn)})
+

JonChesterfield wrote:
> Given that declare variant didn't work elsewhere, it probably doesn't work 
> here. Thus this may be the root cause of 
> https://bugs.llvm.org/show_bug.cgi?id=51337
Was able to reproduce this issue locally on nvptx machine. And you are right, 
declare variant didn't work here as well. Wrapping it in #ifdef fixed the 
issue. I will create a fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104904

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

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp:136
+CheckFactories.registerCheck(
+"readability-variable-length");
   }

Is there a reason this should be restricted to variables? For example, wouldn't 
the same functionality be useful for type names, or dare I say it, even macro 
names? I'm wondering if this should be `readability-identifier-length` to be 
more generic.



Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:20
+
+const unsigned DefaultMinimumVariableNameLength = 3;
+const unsigned DefaultMinimumLoopCounterNameLength = 2;

Should it be possible to enforce parameters differently than local variables? 
(It seems a bit odd that we'd allow you to specify exception variable length 
separate from loops but not function parameters separate from locals.)



Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:24
+const char DefaultIgnoredLoopCounterNames[] = "^[ijk_]$";
+const char DefaultIgnoredVariableNames[] = "";
+

Ignored exception names? There's a fair amount of `catch (const exception &e)` 
code in the world: 
https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=catch%28const+exception+%26e%29&search=Search)



Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:26
+
+const char ErrorMessage[] = "%select{|exception |loop }0 variable name %1 is "
+"too short, expected at least %2 characters";

It looks like there will be whitespace issues with this. If the variable is a 
loop or exception, it'll have an extra space (`loop  variable name`) and if 
it's not, it will start with a leading space (` variable name`).


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

https://reviews.llvm.org/D97753

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

[clang] f3eb5f9 - [AMDGPU][OpenMP] Wrap amdgcn declare variant inside ifdef

[Pushpinder Singh via cfe-commits]

Author: Pushpinder Singh
Date: 2021-08-04T15:24:46Z
New Revision: f3eb5f900d2ae6c8e1c03d1b250415a7b7aa39b1

URL: 
https://github.com/llvm/llvm-project/commit/f3eb5f900d2ae6c8e1c03d1b250415a7b7aa39b1
DIFF: 
https://github.com/llvm/llvm-project/commit/f3eb5f900d2ae6c8e1c03d1b250415a7b7aa39b1.diff

LOG: [AMDGPU][OpenMP] Wrap amdgcn declare variant inside ifdef

This fixes the issue https://bugs.llvm.org/show_bug.cgi?id=51337

Reviewed By: JonChesterfield

Differential Revision: https://reviews.llvm.org/D107468

Added: 


Modified: 
clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h

Removed: 




diff  --git 
a/clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h 
b/clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
index 99cf2483e7343..279fb26fbaf78 100644
--- a/clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
+++ b/clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
@@ -35,6 +35,7 @@ extern "C" {
 
 #pragma omp end declare variant
 
+#ifdef __AMDGCN__
 #pragma omp begin declare variant match(device = {arch(amdgcn)})
 
 // Import types which will be used by __clang_hip_libdevice_declares.h
@@ -54,6 +55,7 @@ extern "C" {
 #undef __OPENMP_AMDGCN__
 
 #pragma omp end declare variant
+#endif
 
 #ifdef __cplusplus
 } // extern "C"



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

[PATCH] D107244: [AIX] Define _ARCH_PPC64 macro for 32-bit

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/PPC.cpp:260
+  } else {
+if (getTriple().isOSAIX())
+  Builder.defineMacro("_ARCH_PPC64");

cebowleratibm wrote:
> A comment is warranted here to explain that __ARCH_PPC64 is only defined in 
> 32-bit on AIX.
s/only/also/; ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107244

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

[PATCH] D107468: [AMDGPU][OpenMP] Wrap amdgcn declare variant inside ifdef

[Pushpinder Singh 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 rGf3eb5f900d2a: [AMDGPU][OpenMP] Wrap amdgcn declare variant 
inside ifdef (authored by pdhaliwal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107468

Files:
  clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h


Index: clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
===
--- clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
+++ clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
@@ -35,6 +35,7 @@
 
 #pragma omp end declare variant
 
+#ifdef __AMDGCN__
 #pragma omp begin declare variant match(device = {arch(amdgcn)})
 
 // Import types which will be used by __clang_hip_libdevice_declares.h
@@ -54,6 +55,7 @@
 #undef __OPENMP_AMDGCN__
 
 #pragma omp end declare variant
+#endif
 
 #ifdef __cplusplus
 } // extern "C"


Index: clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
===
--- clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
+++ clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
@@ -35,6 +35,7 @@
 
 #pragma omp end declare variant
 
+#ifdef __AMDGCN__
 #pragma omp begin declare variant match(device = {arch(amdgcn)})
 
 // Import types which will be used by __clang_hip_libdevice_declares.h
@@ -54,6 +55,7 @@
 #undef __OPENMP_AMDGCN__
 
 #pragma omp end declare variant
+#endif
 
 #ifdef __cplusplus
 } // extern "C"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107243: [AIX] Define __THW_PPC__ macro

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm accepted this revision.
cebowleratibm added a comment.
This revision is now accepted and ready to land.

LGTM.  __THW_PPC__ is a macro historically defined by xlc on AIX and defining 
it may help users port to clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107243

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

[PATCH] D107468: [AMDGPU][OpenMP] Wrap amdgcn declare variant inside ifdef

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield accepted this revision.
JonChesterfield added a comment.
This revision is now accepted and ready to land.

Yes, ship it. We'll look at fixing variant and pull the ifdef back out once 
it's working.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107468

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

[PATCH] D107468: [AMDGPU][OpenMP] Wrap amdgcn declare variant inside ifdef

[Pushpinder Singh via Phabricator via cfe-commits]

pdhaliwal created this revision.
pdhaliwal added reviewers: JonChesterfield, ye-luo, ronlieb.
Herald added subscribers: guansong, t-tye, tpr, dstuttard, yaxunl, kzhuravl.
pdhaliwal requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, wdng.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

This fixes the issue https://bugs.llvm.org/show_bug.cgi?id=51337


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107468

Files:
  clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h


Index: clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
===
--- clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
+++ clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
@@ -35,6 +35,7 @@
 
 #pragma omp end declare variant
 
+#ifdef __AMDGCN__
 #pragma omp begin declare variant match(device = {arch(amdgcn)})
 
 // Import types which will be used by __clang_hip_libdevice_declares.h
@@ -54,6 +55,7 @@
 #undef __OPENMP_AMDGCN__
 
 #pragma omp end declare variant
+#endif
 
 #ifdef __cplusplus
 } // extern "C"


Index: clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
===
--- clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
+++ clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
@@ -35,6 +35,7 @@
 
 #pragma omp end declare variant
 
+#ifdef __AMDGCN__
 #pragma omp begin declare variant match(device = {arch(amdgcn)})
 
 // Import types which will be used by __clang_hip_libdevice_declares.h
@@ -54,6 +55,7 @@
 #undef __OPENMP_AMDGCN__
 
 #pragma omp end declare variant
+#endif
 
 #ifdef __cplusplus
 } // extern "C"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107294: [clang-tidy] adds warning to suggest users replace symbols with words

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.cpp:41-42
+  Finder->addMatcher(
+  binaryOperation(hasAnyOperatorName("&&", "||", "!", "&", "|", "~", "^"))
+  .bind("operator"),
+  this);

`~` and `!` are not binary operations, right?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst:6-7
+
+Finds uses of symbol-based logical and bitwise operators and recommends using
+alternative tokens instead.
+

I strongly recommend splitting this check in two parts:
- Use `and`, `or`, `not` for logical operations.
- Use `bitand`, `bitor`, `xor`, `compl` for non-logical (bitwise) operations.
The reason I recommend this is that I have //seen// people (never in a real 
codebase, mind you, but at least bloggers and such) recommend `and`/`or`/`not` 
for logical operations; this is a long-standing convention in other languages 
such as Python and Perl.  Whereas I have //never// seen anyone recommend e.g. 
`(x bitand compl mask)` over `(x & ~mask)`. So coupling the two ideas together 
seems counterproductive, because //even if// someone wanted to use the Python 
style in their codebase, they wouldn't be able to enforce it using this check, 
because this check would be full of false positives related to the bitwise 
operators.
The current check's recommendation to replace `(a || b) => (a or b)`, `(a ^ b) 
=> (a xor b)` is particularly pernicious to non-language-lawyers, who might 
assume that `or` and `xor` represented the same "part of speech" — either both 
bitwise or both logical. I think this is a major motivation for the Pythonic 
style: use English for logical `and or not`, and use symbols for mathematical 
`& | ^ ~ << >> &= |= ~= <<= >>=`.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst:49-52
+Program composition
+---
+
+This check doesn't yet account for program composition. This means that the

"Program composition" is not a term of art (except in the extremely general 
sense of composing programs, i.e. programming). I recommend retitling this 
section "Use of | as a pipe" or "Use of | with C++20 Ranges".

It might be worth adding a brief mention that the same is true of any 
minigame/DSL embedded within C++; for example, this clang-tidy check will also 
recommend changes like
```
qi::rule(),
ascii::space_type> value = qi::int_ | qi::bool_;
^
warning: use 'bitor' for bitwise disjunctions
```
and
```
std::fstream file("hello.txt", std::ios::in | std::ios::out);
^
warning: use 'bitor' for bitwise disjunctions
```
which are probably not desirable for the user-programmer.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst:58
+
+  // warning: use 'bitor' for logical disjunctions
+  auto evens = std::views::iota(0, 1'000)

s/logical/bitwise/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107294

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks when an FP atomic instruction is converted into a CAS loop or unsafe hardware instruction for GFX90A

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 364132.
gandhi21299 added a comment.

- eliminated the scope argument as per discussion
- added more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx1030.cl
  clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
  clang/test/CodeGenOpenCL/builtins-fp-atomics-unsupported-gfx908.cl

Index: clang/test/CodeGenOpenCL/builtins-fp-atomics-unsupported-gfx908.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/builtins-fp-atomics-unsupported-gfx908.cl
@@ -0,0 +1,8 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -triple amdgcn-amd-amdhsa -target-cpu gfx908 \
+// RUN:   -verify -S -o - %s
+
+void test_global_add(__global double *addr, double x) {
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f64(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fadd_f64' needs target feature gfx90a-insts}}
+}
Index: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
@@ -0,0 +1,106 @@
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -triple amdgcn-amd-amdhsa -target-cpu gfx90a \
+// RUN:   %s -S -emit-llvm -o - | FileCheck %s -check-prefix=CHECK
+
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -triple amdgcn-amd-amdhsa -target-cpu gfx90a \
+// RUN:   -S -o - %s | FileCheck -check-prefix=GFX90A %s
+
+typedef half __attribute__((ext_vector_type(2))) half2;
+
+// CHECK-LABEL: test_global_add
+// CHECK: call double @llvm.amdgcn.global.atomic.fadd.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_global_add$local:
+// GFX90A:  global_atomic_add_f64
+void test_global_add(__global double *addr, double x) {
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f64(addr, x);
+}
+
+// CHECK-LABEL: test_global_global_min
+// CHECK: call double @llvm.amdgcn.global.atomic.fmin.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_global_global_min$local
+// GFX90A:  global_atomic_min_f64
+void test_global_global_min(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fmin_f64(addr, x);
+}
+
+// CHECK-LABEL: test_global_max
+// CHECK: call double @llvm.amdgcn.global.atomic.fmax.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_global_max$local
+// GFX90A:  global_atomic_max_f64
+void test_global_max(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fmax_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_add_local
+// CHECK: call double @llvm.amdgcn.flat.atomic.fadd.f64.p3f64.f64(double addrspace(3)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_add_local$local
+// GFX90A:  ds_add_rtn_f64
+void test_flat_add_local(__local double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fadd_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_global_add
+// CHECK: call double @llvm.amdgcn.flat.atomic.fadd.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_global_add$local
+// GFX90A:  global_atomic_add_f64
+void test_flat_global_add(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fadd_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_min_constant
+// CHECK: call double @llvm.amdgcn.flat.atomic.fmin.f64.p0f64.f64(double* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_min_constant$local
+// GFX90A:  flat_atomic_min_f64
+void test_flat_min_constant(__generic double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fmin_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_global_min
+// CHECK: call double @llvm.amdgcn.flat.atomic.fmin.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A:  test_flat_global_min$local
+// GFX90A:  global_atomic_min_f64
+void test_flat_global_min(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fmin_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_max_constant
+// CHECK: call double @llvm.amdgcn.flat.atomic.fmax.f64.p0f64.f64(double* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_max_constant$local
+// GFX90A:  flat_atomic_max_f64
+void test_flat_max_constant(__generic double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fmax_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_global_max
+// CHECK: call double @llvm.amdgcn.flat.atomic.fmax.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_global_max$local
+// GFX90A:  global_atomic_max_f64
+void test_flat_global_max(__global double *addr, double x

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks when an FP atomic instruction is converted into a CAS loop or unsafe hardware instruction for GFX90A

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:595
+  OptimizationRemark Remark(DEBUG_TYPE, "Passed", RMW->getFunction());
+  Remark << "A hardware instruction was generated";
+  return Remark;

Nothing was generated just yet, pass just left IR instruction untouched. In a 
common case we cannot say what an abstract BE will do about it later.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12139
+OptimizationRemark Remark(DEBUG_TYPE, "Passed", RMW->getFunction());
+Remark << "A hardware instruction was generated";
+return Remark;

It was not generated. We have multiple returns below this point. Some of them 
return None and some CmpXChg for various reasons. The request was to report 
when we produce the instruction *if* it is unsafe, not just that we are about 
to produce an instruction.

Then to make it useful a remark should tell what was the reason to either 
produce an instruction or expand it. Looking at a stream of remarks in a big 
program one would also want to understand what exactly was expanded and what 
was left as is. A stream of messages "A hardware instruction was generated" 
unlikely will help to understand what was done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks when an FP atomic instruction is converted into a CAS loop or unsafe hardware instruction for GFX90A

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16270
+llvm::Function *F = CGM.getIntrinsic(IID, {ArgTy});
+return Builder.CreateCall(F, {Addr, Val, ZeroI32, ZeroI32, ZeroI1});
+  }

@rampitec @arsenm I don't quite remember if I have to do anything here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks when an FP atomic instruction is converted into a CAS loop or unsafe hardware instruction for GFX90A

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

In D106891#2925692 , @gandhi21299 
wrote:

> - eliminated the scope argument as per discussion
> - added more tests

You have updated wrong patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks when an FP atomic instruction is converted into a CAS loop or unsafe hardware instruction for GFX90A

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

JBTW, patch title is way too long.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106960: [OffloadArch] Library to query properties of current offload archicture

[Ye Luo via Phabricator via cfe-commits]

ye-luo added a comment.

I testing with aomp 13.0-5 on ubuntu 20.04.2 LTS (Focal Fossa)

  yeluo@epyc-server:~$ offload-arch -a
  gfx906
  ERROR: offload-arch not found for 10de:2486.
  yeluo@epyc-server:~$ offload-arch -c
  gfx906   sramecc+ xnack-
  yeluo@epyc-server:~$ offload-arch -n
  gfx906 1002:66AF

my second GPU is NVIDIA 3060Ti (sm_86)
I build my app daily with -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_80.

About sm_80 binary able ot run on sm_86
https://docs.nvidia.com/cuda/ampere-compatibility-guide/index.html#application-compatibility-on-ampere


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106960

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

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I see that the UserManual has been updated, but this behavior change should 
probably also be mentioned in the Release Notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74436

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

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 364134.
aaron.ballman added a comment.

More testing fixes


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

https://reviews.llvm.org/D107402

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/aarch64-outliner.c
  clang/test/Driver/aix-object-mode.c
  clang/test/Driver/amdgpu-invalid-target-id.s
  clang/test/Driver/amdgpu-openmp-system-arch-fail.c
  clang/test/Driver/arm-thumb-only-cores.c
  clang/test/Driver/cl-inputs.c
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/cuda-bad-arch.cu
  clang/test/Driver/cuda-detect.cu
  clang/test/Driver/cuda-omp-unsupported-debug-options.cu
  clang/test/Driver/cuda-options-freebsd.cu
  clang/test/Driver/cuda-options.cu
  clang/test/Driver/cuda-version-check.cu
  clang/test/Driver/defsym.s
  clang/test/Driver/fuse-ld.c
  clang/test/Driver/hip-inputs.hip
  clang/test/Driver/hip-invalid-target-id.hip
  clang/test/Driver/invalid-target-id.cl
  clang/test/Driver/msp430-hwmult.c
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/openmp-offload.c
  clang/test/Driver/rocm-detect.cl
  clang/test/Driver/rocm-detect.hip
  clang/test/Driver/rocm-not-found.cl
  clang/test/Frontend/invalid-cxx-abi.cpp
  clang/test/Frontend/round-trip-cc1-args.c
  clang/test/OpenMP/target_messages.cpp

Index: clang/test/OpenMP/target_messages.cpp
===
--- clang/test/OpenMP/target_messages.cpp
+++ clang/test/OpenMP/target_messages.cpp
@@ -8,12 +8,12 @@
 // CHECK: error: OpenMP target is invalid: 'aaa-bbb-ccc-ddd'
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx64-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
-// CHECK-UNSUPPORTED-HOST-TARGET: error: The target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target.
+// CHECK-UNSUPPORTED-HOST-TARGET: error: target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -fopenmp-targets=hexagon-linux-gnu -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-DEVICE-TARGET %s
 // CHECK-UNSUPPORTED-DEVICE-TARGET: OpenMP target is invalid: 'hexagon-linux-gnu'
 
 // RUN: not %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path .bc -o - 2>&1 | FileCheck --check-prefix NO-HOST-BC %s
-// NO-HOST-BC: The provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found.
+// NO-HOST-BC: provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found
 
 // RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm-bc %s -o %t-ppc-host.bc -DREGION_HOST
 // RUN: not %clang_cc1 -verify=expected,omp4 -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -DREGION_DEVICE 2>&1
Index: clang/test/Frontend/round-trip-cc1-args.c
===
--- clang/test/Frontend/round-trip-cc1-args.c
+++ clang/test/Frontend/round-trip-cc1-args.c
@@ -4,4 +4,4 @@
 
 // CHECK-WITHOUT-ROUND-TRIP-NOT: remark:
 // CHECK-ROUND-TRIP-WITHOUT-REMARKS-NOT: remark:
-// CHECK-ROUND-TRIP-WITH-REMARKS: remark: Generated arguments #{{.*}} in round-trip: {{.*}}
+// CHECK-ROUND-TRIP-WITH-REMARKS: remark: generated arguments #{{.*}} in round-trip: {{.*}}
Index: clang/test/Frontend/invalid-cxx-abi.cpp
===
--- clang/test/Frontend/invalid-cxx-abi.cpp
+++ clang/test/Frontend/invalid-cxx-abi.cpp
@@ -1,8 +1,8 @@
 // These shouldn't be valid -fc++-abi values.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=InvalidABI %s 2>&1 | FileCheck %s -check-prefix=INVALID
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=Fuchsia %s 2>&1 | FileCheck %s -check-prefix=CASE-SENSITIVE
-// INVALID: error: Invalid C++ ABI name 'InvalidABI'
-// CASE-SENSITIVE: error: Invalid C++ ABI name 'Fuchsia'
+// INVALID: error: invalid C++ ABI name 'InvalidABI'
+// CASE-SENSITIVE: error: invalid C++ ABI name 'Fuchsia'
 
 // Some C++ ABIs are not supported on some platforms.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=fuchsia -triple i386 %s 2>&1 | FileCheck %s -check-prefix=UNSUPPORTED-FUCHSIA
Index: clang/test/Driver/rocm-not-found.cl
===
--- clang/test/Driver/rocm-not-found.cl
+++ c

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:88
+def err_drv_bad_target_id : Error<
+  "invalid target ID: %0 (a target ID is a processor name followed by an "
+  "optional list of predefined features post-fixed by a plus or minus sign "

aaron.ballman wrote:
> erichkeane wrote:
> >   invalid target ID %0; format is processor name followed by an optional 
> > colon delimited list of features followed by enable/disable sign, .e.g. 
> > 'gfx908:sramecc+;xnack-'
> > 
> > ??  I think we should be leaning on the example more to explain the format.
> I think that's an improvement; the current wording is... hard to interpret. I 
> tweaked it slightly, WDYT?
Erich's suggested wording lacks the colon: `invalid target ID %0`, not `invalid 
target ID: %0`. I agree with Erich's suggestion.
And shouldn't it be `'%0'`, not `%0`, because it's quoting user input? Compare 
lines 96, 98, 206, etc.


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

https://reviews.llvm.org/D107402

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

[clang] e57e1e4 - [clang][AArch64][SVE] Avoid going through memory for fixed/scalable predicate casts

[Bradley Smith via cfe-commits]

Author: Bradley Smith
Date: 2021-08-04T16:10:37Z
New Revision: e57e1e4e00264b77b2b35ad2bf419a48aecdd6bc

URL: 
https://github.com/llvm/llvm-project/commit/e57e1e4e00264b77b2b35ad2bf419a48aecdd6bc
DIFF: 
https://github.com/llvm/llvm-project/commit/e57e1e4e00264b77b2b35ad2bf419a48aecdd6bc.diff

LOG: [clang][AArch64][SVE] Avoid going through memory for fixed/scalable 
predicate casts

For fixed SVE types, predicates are represented using vectors of i8,
where as for scalable types they are represented using vectors of i1. We
can avoid going through memory for casts between these by bitcasting the
i1 scalable vectors to/from a scalable i8 vector of matching size, which
can then use the existing vector insert/extract logic.

Differential Revision: https://reviews.llvm.org/D106860

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGExprScalar.cpp
clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 47a4ed35be85e..1296dfa18b9a5 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1271,12 +1271,26 @@ static llvm::Value *CreateCoercedLoad(Address Src, 
llvm::Type *Ty,
   // perform the conversion.
   if (auto *ScalableDst = dyn_cast(Ty)) {
 if (auto *FixedSrc = dyn_cast(SrcTy)) {
+  // If we are casting a fixed i8 vector to a scalable 16 x i1 predicate
+  // vector, use a vector insert and bitcast the result.
+  bool NeedsBitcast = false;
+  auto PredType =
+  llvm::ScalableVectorType::get(CGF.Builder.getInt1Ty(), 16);
+  llvm::Type *OrigType = Ty;
+  if (ScalableDst == PredType &&
+  FixedSrc->getElementType() == CGF.Builder.getInt8Ty()) {
+ScalableDst = llvm::ScalableVectorType::get(CGF.Builder.getInt8Ty(), 
2);
+NeedsBitcast = true;
+  }
   if (ScalableDst->getElementType() == FixedSrc->getElementType()) {
 auto *Load = CGF.Builder.CreateLoad(Src);
 auto *UndefVec = llvm::UndefValue::get(ScalableDst);
 auto *Zero = llvm::Constant::getNullValue(CGF.CGM.Int64Ty);
-return CGF.Builder.CreateInsertVector(ScalableDst, UndefVec, Load, 
Zero,
-  "castScalableSve");
+llvm::Value *Result = CGF.Builder.CreateInsertVector(
+ScalableDst, UndefVec, Load, Zero, "castScalableSve");
+if (NeedsBitcast)
+  Result = CGF.Builder.CreateBitCast(Result, OrigType);
+return Result;
   }
 }
   }
@@ -2857,9 +2871,18 @@ void CodeGenFunction::EmitFunctionProlog(const 
CGFunctionInfo &FI,
   // llvm.experimental.vector.extract to convert back to the original
   // VLST.
   if (auto *VecTyTo = dyn_cast(ConvertType(Ty))) {
-auto *Coerced = Fn->getArg(FirstIRArg);
+llvm::Value *Coerced = Fn->getArg(FirstIRArg);
 if (auto *VecTyFrom =
 dyn_cast(Coerced->getType())) {
+  // If we are casting a scalable 16 x i1 predicate vector to a fixed 
i8
+  // vector, bitcast the source and use a vector extract.
+  auto PredType =
+  llvm::ScalableVectorType::get(Builder.getInt1Ty(), 16);
+  if (VecTyFrom == PredType &&
+  VecTyTo->getElementType() == Builder.getInt8Ty()) {
+VecTyFrom = llvm::ScalableVectorType::get(Builder.getInt8Ty(), 2);
+Coerced = Builder.CreateBitCast(Coerced, VecTyFrom);
+  }
   if (VecTyFrom->getElementType() == VecTyTo->getElementType()) {
 llvm::Value *Zero = llvm::Constant::getNullValue(CGM.Int64Ty);
 

diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index 418f23bd1a97b..e47701915f2f4 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2063,11 +2063,25 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
 // perform the bitcast.
 if (const auto *FixedSrc = dyn_cast(SrcTy)) {
   if (const auto *ScalableDst = dyn_cast(DstTy)) 
{
+// If we are casting a fixed i8 vector to a scalable 16 x i1 predicate
+// vector, use a vector insert and bitcast the result.
+bool NeedsBitCast = false;
+auto PredType = llvm::ScalableVectorType::get(Builder.getInt1Ty(), 16);
+llvm::Type *OrigType = DstTy;
+if (ScalableDst == PredType &&
+FixedSrc->getElementType() == Builder.getInt8Ty()) {
+  DstTy = llvm::ScalableVectorType::get(Builder.getInt8Ty(), 2);
+  ScalableDst = dyn_cast(DstTy);
+  NeedsBitCast = true;
+}
 if (FixedSrc->getElementType() == Scala

[PATCH] D106860: [clang][AArch64][SVE] Avoid going through memory for fixed/scalable predicate casts

[Bradley Smith 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 rGe57e1e4e0026: [clang][AArch64][SVE] Avoid going through 
memory for fixed/scalable predicate… (authored by bsmith).

Changed prior to commit:
  https://reviews.llvm.org/D106860?vs=363093&id=364153#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106860

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c

Index: clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
@@ -49,20 +49,16 @@
 
 // CHECK-128-LABEL: @write_global_bool(
 // CHECK-128-NEXT:  entry:
-// CHECK-128-NEXT:[[SAVED_VALUE:%.*]] = alloca , align 2
-// CHECK-128-NEXT:store  [[V:%.*]], * [[SAVED_VALUE]], align 2, !tbaa [[TBAA9:![0-9]+]]
-// CHECK-128-NEXT:[[CASTFIXEDSVE:%.*]] = bitcast * [[SAVED_VALUE]] to <2 x i8>*
-// CHECK-128-NEXT:[[TMP0:%.*]] = load <2 x i8>, <2 x i8>* [[CASTFIXEDSVE]], align 2, !tbaa [[TBAA6]]
-// CHECK-128-NEXT:store <2 x i8> [[TMP0]], <2 x i8>* @global_bool, align 2, !tbaa [[TBAA6]]
+// CHECK-128-NEXT:[[TMP0:%.*]] = bitcast  [[V:%.*]] to 
+// CHECK-128-NEXT:[[CASTFIXEDSVE:%.*]] = call <2 x i8> @llvm.experimental.vector.extract.v2i8.nxv2i8( [[TMP0]], i64 0)
+// CHECK-128-NEXT:store <2 x i8> [[CASTFIXEDSVE]], <2 x i8>* @global_bool, align 2, !tbaa [[TBAA6:![0-9]+]]
 // CHECK-128-NEXT:ret void
 //
 // CHECK-512-LABEL: @write_global_bool(
 // CHECK-512-NEXT:  entry:
-// CHECK-512-NEXT:[[SAVED_VALUE:%.*]] = alloca , align 8
-// CHECK-512-NEXT:store  [[V:%.*]], * [[SAVED_VALUE]], align 8, !tbaa [[TBAA9:![0-9]+]]
-// CHECK-512-NEXT:[[CASTFIXEDSVE:%.*]] = bitcast * [[SAVED_VALUE]] to <8 x i8>*
-// CHECK-512-NEXT:[[TMP0:%.*]] = load <8 x i8>, <8 x i8>* [[CASTFIXEDSVE]], align 8, !tbaa [[TBAA6]]
-// CHECK-512-NEXT:store <8 x i8> [[TMP0]], <8 x i8>* @global_bool, align 2, !tbaa [[TBAA6]]
+// CHECK-512-NEXT:[[TMP0:%.*]] = bitcast  [[V:%.*]] to 
+// CHECK-512-NEXT:[[CASTFIXEDSVE:%.*]] = call <8 x i8> @llvm.experimental.vector.extract.v8i8.nxv2i8( [[TMP0]], i64 0)
+// CHECK-512-NEXT:store <8 x i8> [[CASTFIXEDSVE]], <8 x i8>* @global_bool, align 2, !tbaa [[TBAA6]]
 // CHECK-512-NEXT:ret void
 //
 void write_global_bool(svbool_t v) { global_bool = v; }
@@ -101,20 +97,16 @@
 
 // CHECK-128-LABEL: @read_global_bool(
 // CHECK-128-NEXT:  entry:
-// CHECK-128-NEXT:[[SAVED_VALUE:%.*]] = alloca <2 x i8>, align 2
 // CHECK-128-NEXT:[[TMP0:%.*]] = load <2 x i8>, <2 x i8>* @global_bool, align 2, !tbaa [[TBAA6]]
-// CHECK-128-NEXT:store <2 x i8> [[TMP0]], <2 x i8>* [[SAVED_VALUE]], align 2, !tbaa [[TBAA6]]
-// CHECK-128-NEXT:[[CASTFIXEDSVE:%.*]] = bitcast <2 x i8>* [[SAVED_VALUE]] to *
-// CHECK-128-NEXT:[[TMP1:%.*]] = load , * [[CASTFIXEDSVE]], align 2, !tbaa [[TBAA6]]
+// CHECK-128-NEXT:[[CASTFIXEDSVE:%.*]] = call  @llvm.experimental.vector.insert.nxv2i8.v2i8( undef, <2 x i8> [[TMP0]], i64 0)
+// CHECK-128-NEXT:[[TMP1:%.*]] = bitcast  [[CASTFIXEDSVE]] to 
 // CHECK-128-NEXT:ret  [[TMP1]]
 //
 // CHECK-512-LABEL: @read_global_bool(
 // CHECK-512-NEXT:  entry:
-// CHECK-512-NEXT:[[SAVED_VALUE:%.*]] = alloca <8 x i8>, align 8
 // CHECK-512-NEXT:[[TMP0:%.*]] = load <8 x i8>, <8 x i8>* @global_bool, align 2, !tbaa [[TBAA6]]
-// CHECK-512-NEXT:store <8 x i8> [[TMP0]], <8 x i8>* [[SAVED_VALUE]], align 8, !tbaa [[TBAA6]]
-// CHECK-512-NEXT:[[CASTFIXEDSVE:%.*]] = bitcast <8 x i8>* [[SAVED_VALUE]] to *
-// CHECK-512-NEXT:[[TMP1:%.*]] = load , * [[CASTFIXEDSVE]], align 8, !tbaa [[TBAA6]]
+// CHECK-512-NEXT:[[CASTFIXEDSVE:%.*]] = call  @llvm.experimental.vector.insert.nxv2i8.v8i8( undef, <8 x i8> [[TMP0]], i64 0)
+// CHECK-512-NEXT:[[TMP1:%.*]] = bitcast  [[CASTFIXEDSVE]] to 
 // CHECK-512-NEXT:ret  [[TMP1]]
 //
 svbool_t read_global_bool() { return global_bool; }
Index: clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
@@ -18,19 +18,15 @@
 // CHECK-NEXT:[[PRED_ADDR:%.*]] = alloca , align 2
 // CHECK-NEXT:[[VEC_ADDR:%.*]] = alloca , align 16
 // CHECK-NEXT:[[PG:%.*]] = alloca , align 2
-// CHECK-NEXT:[[SAVED_VALUE:%.*]] = alloca <8 x i8>, align 8
-// CHECK-NEXT:[[SAVED_VALUE1:%.*]] = alloca <8 x i8>, align 8
 // C

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:88
+def err_drv_bad_target_id : Error<
+  "invalid target ID: %0 (a target ID is a processor name followed by an "
+  "optional list of predefined features post-fixed by a plus or minus sign "

Quuxplusone wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > >   invalid target ID %0; format is processor name followed by an optional 
> > > colon delimited list of features followed by enable/disable sign, .e.g. 
> > > 'gfx908:sramecc+;xnack-'
> > > 
> > > ??  I think we should be leaning on the example more to explain the 
> > > format.
> > I think that's an improvement; the current wording is... hard to interpret. 
> > I tweaked it slightly, WDYT?
> Erich's suggested wording lacks the colon: `invalid target ID %0`, not 
> `invalid target ID: %0`. I agree with Erich's suggestion.
> And shouldn't it be `'%0'`, not `%0`, because it's quoting user input? 
> Compare lines 96, 98, 206, etc.
Ah, right!  I missed that in review.  Yes, I think the colon doesn't add 
anything/should be removed.  I also agree about quoting it.


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

https://reviews.llvm.org/D107402

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

[PATCH] D107420: [WIP][sema] Disallow __builtin_mul_overflow under special condition.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe updated this revision to Diff 364154.
FreddyYe marked 2 inline comments as done.
FreddyYe added a comment.

Address comments. I'll refactor clang-format later. Pls help review the new 
condition and diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107420

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/builtins-overflow.c


Index: clang/test/Sema/builtins-overflow.c
===
--- clang/test/Sema/builtins-overflow.c
+++ clang/test/Sema/builtins-overflow.c
@@ -38,4 +38,10 @@
 _ExtInt(129) result;
 _Bool status = __builtin_mul_overflow(x, y, &result); // expected-error 
{{__builtin_mul_overflow does not support signed _ExtInt operands of more than 
128 bits}}
   }
+  {
+_ExtInt(128) x = 1;
+_ExtInt(128) y = 1;
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, &result); // expected-error 
{{__builtin_mul_overflow does not support special combination operands (signed, 
signed, unsigned*) of more than 64 bits}}
+  }
 }
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -328,6 +328,20 @@
   // Disallow signed ExtIntType args larger than 128 bits to mul function until
   // we improve backend support.
   if (BuiltinID == Builtin::BI__builtin_mul_overflow) {
+const auto LeftTy = TheCall->getArg(0)->getType();
+const auto RightTy = TheCall->getArg(1)->getType();
+const auto ResultPointeeTy = 
TheCall->getArg(2)->getType()->getPointeeType();
+// Input combination below will also emit an integer value larger than
+// 128 bits in the backend, disallow same as above.
+if (!ResultTy->isSignedIntegerType() &&
+S.getASTContext().getIntWidth(ResultTy) >= 128 &&
+(LeftTy->isSignedIntegerType() || RightTy->isSignedIntegerType()) &&
+(S.getASTContext().getIntWidth(LeftTy) + 
S.getASTContext().getIntWidth(RightTy)) > 128 &&
+) {
+  return S.Diag(TheCall->getArg(0)->getBeginLoc(),
+diag::err_overflow_builtin_special_combination_max_size)
+ << 127;
+}
 for (unsigned I = 0; I < 3; ++I) {
   const auto Arg = TheCall->getArg(I);
   // Third argument will be a pointer.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8348,6 +8348,9 @@
 def err_overflow_builtin_ext_int_max_size : Error<
   "__builtin_mul_overflow does not support signed _ExtInt operands of more "
   "than %0 bits">;
+def err_overflow_builtin_special_combination_max_size : Error<
+  "__builtin_mul_overflow does not suport unsigned overflow check after 
convention "
+  "more than %0 bits">;
 
 def err_atomic_load_store_uses_lib : Error<
   "atomic %select{load|store}0 requires runtime support that is not "


Index: clang/test/Sema/builtins-overflow.c
===
--- clang/test/Sema/builtins-overflow.c
+++ clang/test/Sema/builtins-overflow.c
@@ -38,4 +38,10 @@
 _ExtInt(129) result;
 _Bool status = __builtin_mul_overflow(x, y, &result); // expected-error {{__builtin_mul_overflow does not support signed _ExtInt operands of more than 128 bits}}
   }
+  {
+_ExtInt(128) x = 1;
+_ExtInt(128) y = 1;
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, &result); // expected-error {{__builtin_mul_overflow does not support special combination operands (signed, signed, unsigned*) of more than 64 bits}}
+  }
 }
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -328,6 +328,20 @@
   // Disallow signed ExtIntType args larger than 128 bits to mul function until
   // we improve backend support.
   if (BuiltinID == Builtin::BI__builtin_mul_overflow) {
+const auto LeftTy = TheCall->getArg(0)->getType();
+const auto RightTy = TheCall->getArg(1)->getType();
+const auto ResultPointeeTy = TheCall->getArg(2)->getType()->getPointeeType();
+// Input combination below will also emit an integer value larger than
+// 128 bits in the backend, disallow same as above.
+if (!ResultTy->isSignedIntegerType() &&
+S.getASTContext().getIntWidth(ResultTy) >= 128 &&
+(LeftTy->isSignedIntegerType() || RightTy->isSignedIntegerType()) &&
+(S.getASTContext().getIntWidth(LeftTy) + S.getASTContext().getIntWidth(RightTy)) > 128 &&
+) {
+  return S.Diag(TheCall->getArg(0)->getBeginLoc(),
+diag::err_overflow_builtin_specia

[PATCH] D107420: [sema] Disallow __builtin_mul_overflow under special condition.

[Freddy, Ye via Phabricator via cfe-commits]

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

Addressed. THX review!




Comment at: clang/test/Sema/builtins-overflow.c:45
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, &result); // expected-error 
{{__builtin_mul_overflow does not support special combination operands (signed, 
signed, unsigned*) of more than 64 bits}}
+  }

aaron.ballman wrote:
> Yeah, this diagnostic really doesn't tell me what's going wrong with the code 
> or how to fix it. Do we basically want to prevent using larger-than-64-bit 
> argument types with mixed signs? Or are there other problematic circumstances?
Yes, let me try to refine. I can explain more what happened to such input 
combination.
According to gcc's the definition on this builtin: 
https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html
'These built-in functions promote the first two operands into infinite 
precision signed type and perform multiply on those promoted operands. The 
result is then cast to the type the third pointer argument points to and stored 
there.' 

Since signing integer has a smaller range than unsigned integer. And now the 
API in compiler-rt (`__muloti4`) to checking 128 integer's multiplying is 
implemented in signed version. So the overflow max absolute value it can check 
is 2^127. When the result input is larger equal than 128 bits, `__muloti4` has 
no usage. We should prevent this situation for now. Or the backend will crush 
as the example shows.

I found the input operand doesn't need both of them larger than 64 bits, but 
just the sum of their larger 128. I'll refine in my patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107420

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

[PATCH] D107420: [sema] Disallow __builtin_mul_overflow under special condition.

[Freddy, Ye via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8351-8353
+def err_overflow_builtin_special_combination_max_size : Error<
+  "__builtin_mul_overflow does not suport unsigned overflow check after 
convention "
+  "more than %0 bits">;

The new diagnostic is here. Forgot to update tests since I've not rebuilt 
completely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107420

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

[PATCH] D94639: [DebugInfo][CodeView] Change in line tables only mode to emit parent/context scopes for functions, using declarations for types

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D94639#2918559 , @SeTSeR wrote:

> Thank you for the detailed answer. I'll discuss our use case with our team. 
> Should I create a separate ticket for this? Or maybe it would be better if I 
> submitted the PR adding this flag?

It's free to upload patches, and they help drive discussion forward, but I 
think the main thing is to be really clear about what problem this solves and 
what the costs are. An easy metric for the cost would be the binary size of 
clang with `-g1` and how much it increases in the new mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94639

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

[PATCH] D107477: [LLVM][AST][NFC] Resolve Fixme: Make CXXRecordDecl *Record const.

[Alf via Phabricator via cfe-commits]

gAlfonso-bit created this revision.
gAlfonso-bit added a project: LLVM.
gAlfonso-bit requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107477

Files:
  clang/include/clang/AST/ComparisonCategories.h


Index: clang/include/clang/AST/ComparisonCategories.h
===
--- clang/include/clang/AST/ComparisonCategories.h
+++ clang/include/clang/AST/ComparisonCategories.h
@@ -115,8 +115,7 @@
 public:
   /// The declaration for the comparison category type from the
   /// standard library.
-  // FIXME: Make this const
-  CXXRecordDecl *Record = nullptr;
+  const CXXRecordDecl *Record = nullptr;
 
   /// The Kind of the comparison category type
   ComparisonCategoryType Kind;


Index: clang/include/clang/AST/ComparisonCategories.h
===
--- clang/include/clang/AST/ComparisonCategories.h
+++ clang/include/clang/AST/ComparisonCategories.h
@@ -115,8 +115,7 @@
 public:
   /// The declaration for the comparison category type from the
   /// standard library.
-  // FIXME: Make this const
-  CXXRecordDecl *Record = nullptr;
+  const CXXRecordDecl *Record = nullptr;
 
   /// The Kind of the comparison category type
   ComparisonCategoryType Kind;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106909: [clang] Add clang builtins support for gfx90a

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 364152.
gandhi21299 added a comment.

updating test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106909

Files:
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx1030.cl
  clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
  clang/test/CodeGenOpenCL/builtins-fp-atomics-unsupported-gfx908.cl

Index: clang/test/CodeGenOpenCL/builtins-fp-atomics-unsupported-gfx908.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/builtins-fp-atomics-unsupported-gfx908.cl
@@ -0,0 +1,8 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -triple amdgcn-amd-amdhsa -target-cpu gfx908 \
+// RUN:   -verify -S -o - %s
+
+void test_global_add(__global double *addr, double x) {
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f64(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fadd_f64' needs target feature gfx90a-insts}}
+}
Index: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -triple amdgcn-amd-amdhsa -target-cpu gfx90a \
+// RUN:   %s -S -emit-llvm -o - | FileCheck %s -check-prefix=CHECK
+
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -triple amdgcn-amd-amdhsa -target-cpu gfx90a \
+// RUN:   -S -o - %s | FileCheck -check-prefix=GFX90A %s
+
+typedef half __attribute__((ext_vector_type(2))) half2;
+
+// CHECK-LABEL: test_global_add
+// CHECK: call double @llvm.amdgcn.global.atomic.fadd.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_global_add$local:
+// GFX90A:  global_atomic_add_f64
+void test_global_add(__global double *addr, double x) {
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f64(addr, x);
+}
+
+// CHECK-LABEL: test_global_add_half2
+// CHECK: call <2 x half> @llvm.amdgcn.global.atomic.fadd.v2f16.p1v2f16.v2f16(<2 x half> addrspace(1)* %{{.*}}, <2 x half> %{{.*}})
+// GFX90A-LABEL:  test_global_add_half2
+// GFX90A:  global_atomic_pk_add_f16 v2, v[0:1], v2, off glc
+void test_global_add_half2(__global half2 *addr, half2 x) {
+  half2 *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fadd_v2f16(addr, x);
+}
+
+// CHECK-LABEL: test_global_global_min
+// CHECK: call double @llvm.amdgcn.global.atomic.fmin.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_global_global_min$local
+// GFX90A:  global_atomic_min_f64
+void test_global_global_min(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fmin_f64(addr, x);
+}
+
+// CHECK-LABEL: test_global_max
+// CHECK: call double @llvm.amdgcn.global.atomic.fmax.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_global_max$local
+// GFX90A:  global_atomic_max_f64
+void test_global_max(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fmax_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_add_local
+// CHECK: call double @llvm.amdgcn.flat.atomic.fadd.f64.p3f64.f64(double addrspace(3)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_add_local$local
+// GFX90A:  ds_add_rtn_f64
+void test_flat_add_local(__local double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fadd_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_global_add
+// CHECK: call double @llvm.amdgcn.flat.atomic.fadd.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_global_add$local
+// GFX90A:  global_atomic_add_f64
+void test_flat_global_add(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fadd_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_min_constant
+// CHECK: call double @llvm.amdgcn.flat.atomic.fmin.f64.p0f64.f64(double* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_min_constant$local
+// GFX90A:  flat_atomic_min_f64
+void test_flat_min_constant(__generic double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fmin_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_global_min
+// CHECK: call double @llvm.amdgcn.flat.atomic.fmin.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A:  test_flat_global_min$local
+// GFX90A:  global_atomic_min_f64
+void test_flat_global_min(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fmin_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_max_constant
+// CHECK: call double @llvm.amdgcn.flat.atomic.fmax.f64.p0f64.f64(double* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_max_constant$local
+// GFX90A:  flat_atomic_max_f64
+void test_flat_max_constant(__generic double *addr, double x){
+  dou

[PATCH] D106909: [clang] Add clang builtins support for gfx90a

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsAMDGPU.def:201
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_f32, "ff*1f", "t", 
"gfx90a-insts")
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_v2f16, "V2hV2h*1V2h", "t", 
"gfx90a-insts")
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fmin_f64, "dd*1d", "t", 
"gfx90a-insts")

I tried add target feature gfx908-insts for this builtin but the frontend 
complains that it should have target feature gfx90a-insts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106909

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

[PATCH] D107241: [AIX] Define __THW_BIG_ENDIAN__ macro

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm accepted this revision.
cebowleratibm added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107241

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

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

[David Goldman via Phabricator via cfe-commits]

dgoldman marked 2 inline comments as done.
dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:664
+  }
+  // NextPragma is after us but before the next symbol. Our reign is over.
+  break;

kadircet wrote:
> I suppose this is not correct in cases like.
> 
> ```
> #pragma mark - Outer
> foo {
>   #pragma mark - Foo
> }
> bar {
>   # pragma mark - Bar
> }
> baz {
>   # pragma mark - Baz
> }
> ```
> 
> We'll first move `foo` under `Outer`, then start processing `bar` we notice 
> that It comes after pragma `Foo`, and then also notice that pragma `Foo` is 
> not contained in `bar` hence break. but all of `foo,bar,baz` should've been 
> moved to be under `Outer` instead.
Yeah that's a good point, we'd have to do something like your suggested 
solution or maintain more state to consider the children that were moved.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:535
+/// by range.
+std::vector mergePragmas(std::vector &Syms,
+ std::vector 
&Pragmas,

kadircet wrote:
> dgoldman wrote:
> > kadircet wrote:
> > > dgoldman wrote:
> > > > kadircet wrote:
> > > > > dgoldman wrote:
> > > > > > kadircet wrote:
> > > > > > > dgoldman wrote:
> > > > > > > > sammccall wrote:
> > > > > > > > > FWIW the flow control/how we make progress seem hard to 
> > > > > > > > > follow here to me.
> > > > > > > > > 
> > > > > > > > > In particular I think I'm struggling with the statefulness of 
> > > > > > > > > "is there an open mark group".
> > > > > > > > > 
> > > > > > > > > Possible simplifications:
> > > > > > > > >  - define a dummy root symbol, which seems clearer than the 
> > > > > > > > > vector + range
> > > > > > > > >  - avoid reverse-sorting the list of pragma symbols, and just 
> > > > > > > > > consume from the front of an ArrayRef instead
> > > > > > > > >  - make the outer loop over pragmas, rather than symbols. It 
> > > > > > > > > would first check if the pragma belongs directly here or not, 
> > > > > > > > > and if so, loop over symbols to work out which should become 
> > > > > > > > > children. This seems very likely to be efficient enough in 
> > > > > > > > > practice (few pragmas, or most children are grouped into 
> > > > > > > > > pragmas)
> > > > > > > > > define a dummy root symbol, which seems clearer than the 
> > > > > > > > > vector + range
> > > > > > > > 
> > > > > > > > I guess? Then we'd take in a `DocumentSymbol & and a 
> > > > > > > > ArrayRef & (or just by value and then return 
> > > > > > > > it as well). The rest would be the same though
> > > > > > > > 
> > > > > > > > > In particular I think I'm struggling with the statefulness of 
> > > > > > > > > "is there an open mark group".
> > > > > > > > 
> > > > > > > > We need to track the current open group if there is one in 
> > > > > > > > order to move children to it.
> > > > > > > > 
> > > > > > > > > make the outer loop over pragmas, rather than symbols. It 
> > > > > > > > > would first check if the pragma belongs directly here or not, 
> > > > > > > > > and if so, loop over symbols to work out which should become 
> > > > > > > > > children. This seems very likely to be efficient enough in 
> > > > > > > > > practice (few pragmas, or most children are grouped into 
> > > > > > > > > pragmas)
> > > > > > > > 
> > > > > > > > The important thing here is knowing where the pragma mark ends 
> > > > > > > > - if it doesn't, it actually gets all of the children. So we'd 
> > > > > > > > have to peak at the next pragma mark, add all symbols before it 
> > > > > > > > to us as children, and then potentially recurse to nest it 
> > > > > > > > inside of a symbol. I'll try it out and see if it's simpler.
> > > > > > > > 
> > > > > > > > 
> > > > > > > ```
> > > > > > > while(Pragmas) {
> > > > > > > // We'll figure out where the Pragmas.front() should go.
> > > > > > > Pragma P = Pragmas.front();
> > > > > > > DocumentSymbol *Cur = Root;
> > > > > > > while(Cur->contains(P)) {
> > > > > > >   auto *OldCur = Cur;
> > > > > > >   for(auto *C : Cur->children) {
> > > > > > >  // We assume at most 1 child can contain the pragma (as 
> > > > > > > pragmas are on a single line, and children have disjoint ranges)
> > > > > > >  if (C->contains(P)) {
> > > > > > >  Cur = C;
> > > > > > >  break;
> > > > > > >  }
> > > > > > >   }
> > > > > > >   // Cur is immediate parent of P
> > > > > > >   if (OldCur == Cur) {
> > > > > > > // Just insert P into children if it is not a group and we 
> > > > > > > are done.
> > > > > > > // Otherwise we need to figure out when current pragma is 
> > > > > > > terminated:
> > > > > > > // if next pragma is not contained in Cur, or is contained in one 
> > > > > > > of the children, It is at the end of Cur, nest all the children 
> > > > > > > that appear after P under the symbol node for P.
> > > > > > > // Otherwise nest a

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

[David Goldman via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:535
+/// by range.
+std::vector mergePragmas(std::vector &Syms,
+ std::vector 
&Pragmas,

dgoldman wrote:
> kadircet wrote:
> > dgoldman wrote:
> > > kadircet wrote:
> > > > dgoldman wrote:
> > > > > kadircet wrote:
> > > > > > dgoldman wrote:
> > > > > > > kadircet wrote:
> > > > > > > > dgoldman wrote:
> > > > > > > > > sammccall wrote:
> > > > > > > > > > FWIW the flow control/how we make progress seem hard to 
> > > > > > > > > > follow here to me.
> > > > > > > > > > 
> > > > > > > > > > In particular I think I'm struggling with the statefulness 
> > > > > > > > > > of "is there an open mark group".
> > > > > > > > > > 
> > > > > > > > > > Possible simplifications:
> > > > > > > > > >  - define a dummy root symbol, which seems clearer than the 
> > > > > > > > > > vector + range
> > > > > > > > > >  - avoid reverse-sorting the list of pragma symbols, and 
> > > > > > > > > > just consume from the front of an ArrayRef instead
> > > > > > > > > >  - make the outer loop over pragmas, rather than symbols. 
> > > > > > > > > > It would first check if the pragma belongs directly here or 
> > > > > > > > > > not, and if so, loop over symbols to work out which should 
> > > > > > > > > > become children. This seems very likely to be efficient 
> > > > > > > > > > enough in practice (few pragmas, or most children are 
> > > > > > > > > > grouped into pragmas)
> > > > > > > > > > define a dummy root symbol, which seems clearer than the 
> > > > > > > > > > vector + range
> > > > > > > > > 
> > > > > > > > > I guess? Then we'd take in a `DocumentSymbol & and a 
> > > > > > > > > ArrayRef & (or just by value and then 
> > > > > > > > > return it as well). The rest would be the same though
> > > > > > > > > 
> > > > > > > > > > In particular I think I'm struggling with the statefulness 
> > > > > > > > > > of "is there an open mark group".
> > > > > > > > > 
> > > > > > > > > We need to track the current open group if there is one in 
> > > > > > > > > order to move children to it.
> > > > > > > > > 
> > > > > > > > > > make the outer loop over pragmas, rather than symbols. It 
> > > > > > > > > > would first check if the pragma belongs directly here or 
> > > > > > > > > > not, and if so, loop over symbols to work out which should 
> > > > > > > > > > become children. This seems very likely to be efficient 
> > > > > > > > > > enough in practice (few pragmas, or most children are 
> > > > > > > > > > grouped into pragmas)
> > > > > > > > > 
> > > > > > > > > The important thing here is knowing where the pragma mark 
> > > > > > > > > ends - if it doesn't, it actually gets all of the children. 
> > > > > > > > > So we'd have to peak at the next pragma mark, add all symbols 
> > > > > > > > > before it to us as children, and then potentially recurse to 
> > > > > > > > > nest it inside of a symbol. I'll try it out and see if it's 
> > > > > > > > > simpler.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > ```
> > > > > > > > while(Pragmas) {
> > > > > > > > // We'll figure out where the Pragmas.front() should go.
> > > > > > > > Pragma P = Pragmas.front();
> > > > > > > > DocumentSymbol *Cur = Root;
> > > > > > > > while(Cur->contains(P)) {
> > > > > > > >   auto *OldCur = Cur;
> > > > > > > >   for(auto *C : Cur->children) {
> > > > > > > >  // We assume at most 1 child can contain the pragma (as 
> > > > > > > > pragmas are on a single line, and children have disjoint ranges)
> > > > > > > >  if (C->contains(P)) {
> > > > > > > >  Cur = C;
> > > > > > > >  break;
> > > > > > > >  }
> > > > > > > >   }
> > > > > > > >   // Cur is immediate parent of P
> > > > > > > >   if (OldCur == Cur) {
> > > > > > > > // Just insert P into children if it is not a group and we 
> > > > > > > > are done.
> > > > > > > > // Otherwise we need to figure out when current pragma is 
> > > > > > > > terminated:
> > > > > > > > // if next pragma is not contained in Cur, or is contained in 
> > > > > > > > one of the children, It is at the end of Cur, nest all the 
> > > > > > > > children that appear after P under the symbol node for P.
> > > > > > > > // Otherwise nest all the children that appear after P but 
> > > > > > > > before next pragma under the symbol node for P.
> > > > > > > > // Pop Pragmas and break
> > > > > > > >   }
> > > > > > > > }
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > Does that make sense, i hope i am not missing something 
> > > > > > > > obvious? Complexity-wise in the worst case we'll go all the way 
> > > > > > > > down to a leaf once per pragma, since there will only be a 
> > > > > > > > handful of pragmas most of the time it shouldn't be too bad.
> > > > > > > I've implemented your suggestion. I don't think it's simpler, but 
> > > > > 

[PATCH] D107394: [AIX] "aligned" attribute does not decrease alignment

[Sean Fertile via Phabricator via cfe-commits]

sfertile added inline comments.



Comment at: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp:11
+
+// CHECK: @c = global %struct.C zeroinitializer, align 2

Minor nit: I think the other test can live in this file, highlighting the 
difference between the 2 cases is nice. Also I think we should add a typedef 
test in here as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

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

[PATCH] D106909: [clang] Add clang builtins support for gfx90a

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsAMDGPU.def:201
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_f32, "ff*1f", "t", 
"gfx90a-insts")
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_v2f16, "V2hV2h*1V2h", "t", 
"gfx90a-insts")
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fmin_f64, "dd*1d", "t", 
"gfx90a-insts")

gandhi21299 wrote:
> I tried add target feature gfx908-insts for this builtin but the frontend 
> complains that it should have target feature gfx90a-insts.
That was for global_atomic_fadd_f32, but as per discussion we are going to use 
builtin only starting from gfx90a because of the noret problem. Comments in the 
review are off their positions after multiple patch updates.



Comment at: clang/include/clang/Basic/BuiltinsAMDGPU.def:210
+TARGET_BUILTIN(__builtin_amdgcn_ds_atomic_fadd_f64, "dd*3d", "t", 
"gfx90a-insts")
+TARGET_BUILTIN(__builtin_amdgcn_ds_atomic_fadd_f32, "ff*3f", "t", "gfx8-insts")
+

This needs tests with a gfx8 target and a negative test with gfx7.



Comment at: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl:13
+// GFX90A:  global_atomic_add_f64
+void test_global_add(__global double *addr, double x) {
+  double *rtn;

Use _f64 or _double in the test name.



Comment at: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl:31
+// GFX90A:  global_atomic_min_f64
+void test_global_global_min(__global double *addr, double x){
+  double *rtn;

Same here and in other double tests, use a suffix f64 or double.



Comment at: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl:67
+// GFX90A:  flat_atomic_min_f64
+void test_flat_min_constant(__generic double *addr, double x){
+  double *rtn;

'constant' is wrong. It is flat. Here and everywhere.



Comment at: clang/test/CodeGenOpenCL/builtins-fp-atomics-unsupported-gfx908.cl:7
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f64(addr, x); // 
expected-error{{'__builtin_amdgcn_global_atomic_fadd_f64' needs target feature 
gfx90a-insts}}
+}

Need to check all other buintins too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106909

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

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 364172.
aaron.ballman added a comment.

Updating based on review feedback.


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

https://reviews.llvm.org/D107402

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/aarch64-outliner.c
  clang/test/Driver/aix-object-mode.c
  clang/test/Driver/amdgpu-invalid-target-id.s
  clang/test/Driver/amdgpu-openmp-system-arch-fail.c
  clang/test/Driver/arm-thumb-only-cores.c
  clang/test/Driver/cl-inputs.c
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/cuda-bad-arch.cu
  clang/test/Driver/cuda-detect.cu
  clang/test/Driver/cuda-omp-unsupported-debug-options.cu
  clang/test/Driver/cuda-options-freebsd.cu
  clang/test/Driver/cuda-options.cu
  clang/test/Driver/cuda-version-check.cu
  clang/test/Driver/defsym.s
  clang/test/Driver/fuse-ld.c
  clang/test/Driver/hip-inputs.hip
  clang/test/Driver/hip-invalid-target-id.hip
  clang/test/Driver/invalid-target-id.cl
  clang/test/Driver/msp430-hwmult.c
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/openmp-offload.c
  clang/test/Driver/rocm-detect.cl
  clang/test/Driver/rocm-detect.hip
  clang/test/Driver/rocm-not-found.cl
  clang/test/Frontend/invalid-cxx-abi.cpp
  clang/test/Frontend/round-trip-cc1-args.c
  clang/test/OpenMP/target_messages.cpp

Index: clang/test/OpenMP/target_messages.cpp
===
--- clang/test/OpenMP/target_messages.cpp
+++ clang/test/OpenMP/target_messages.cpp
@@ -8,12 +8,12 @@
 // CHECK: error: OpenMP target is invalid: 'aaa-bbb-ccc-ddd'
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx64-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
-// CHECK-UNSUPPORTED-HOST-TARGET: error: The target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target.
+// CHECK-UNSUPPORTED-HOST-TARGET: error: target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -fopenmp-targets=hexagon-linux-gnu -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-DEVICE-TARGET %s
 // CHECK-UNSUPPORTED-DEVICE-TARGET: OpenMP target is invalid: 'hexagon-linux-gnu'
 
 // RUN: not %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path .bc -o - 2>&1 | FileCheck --check-prefix NO-HOST-BC %s
-// NO-HOST-BC: The provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found.
+// NO-HOST-BC: provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found
 
 // RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm-bc %s -o %t-ppc-host.bc -DREGION_HOST
 // RUN: not %clang_cc1 -verify=expected,omp4 -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -DREGION_DEVICE 2>&1
Index: clang/test/Frontend/round-trip-cc1-args.c
===
--- clang/test/Frontend/round-trip-cc1-args.c
+++ clang/test/Frontend/round-trip-cc1-args.c
@@ -4,4 +4,4 @@
 
 // CHECK-WITHOUT-ROUND-TRIP-NOT: remark:
 // CHECK-ROUND-TRIP-WITHOUT-REMARKS-NOT: remark:
-// CHECK-ROUND-TRIP-WITH-REMARKS: remark: Generated arguments #{{.*}} in round-trip: {{.*}}
+// CHECK-ROUND-TRIP-WITH-REMARKS: remark: generated arguments #{{.*}} in round-trip: {{.*}}
Index: clang/test/Frontend/invalid-cxx-abi.cpp
===
--- clang/test/Frontend/invalid-cxx-abi.cpp
+++ clang/test/Frontend/invalid-cxx-abi.cpp
@@ -1,8 +1,8 @@
 // These shouldn't be valid -fc++-abi values.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=InvalidABI %s 2>&1 | FileCheck %s -check-prefix=INVALID
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=Fuchsia %s 2>&1 | FileCheck %s -check-prefix=CASE-SENSITIVE
-// INVALID: error: Invalid C++ ABI name 'InvalidABI'
-// CASE-SENSITIVE: error: Invalid C++ ABI name 'Fuchsia'
+// INVALID: error: invalid C++ ABI name 'InvalidABI'
+// CASE-SENSITIVE: error: invalid C++ ABI name 'Fuchsia'
 
 // Some C++ ABIs are not supported on some platforms.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=fuchsia -triple i386 %s 2>&1 | FileCheck %s -check-prefix=UNSUPPORTED-FUCHSIA
Index: clang/test/Driver/rocm-not-found.cl
===
--- clang/test/Driver/rocm-no

[PATCH] D106960: [OffloadArch] Library to query properties of current offload archicture

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D106960#2925610 , @ye-luo wrote:

> my second GPU is NVIDIA 3060Ti (sm_86)
> I build my app daily with -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_80.
>
> About sm_80 binary able ot run on sm_86
> https://docs.nvidia.com/cuda/ampere-compatibility-guide/index.html#application-compatibility-on-ampere

Keep in mind that the binaries compiled for sm_80 will likely run a lot slower 
on sm_86. sm_86 has distinctly different hardware and the code generated for 
sm_80 will be sub-optimal for it.
I don't have the Ampere cards to compare, but sm_70 binaries running on sm_75 
were reached only about 1/2 of the speed of the same code compiled for sm_75 
when it was operating on fp16.

NVIDIA didn't provide performance tuning guide for Ampere, but here's what it 
had to say about Volta/Turing:
https://docs.nvidia.com/cuda/turing-tuning-guide/index.html#tensor-operations

> Any binary compiled for Volta will run on Turing, but Volta binaries using 
> Tensor Cores will only be able to reach half of Turing's Tensor Core peak 
> performance. 
> Recompiling the binary specifically for Turing would allow it to reach the 
> peak performance.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106960

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

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D107304#2924886 , @kadircet wrote:

> Ok if you think that setting GCC version to higher values by default won't 
> break anything,

I think that GCC version should be the same, which was used to build the 
project: it can be higher or lower than 4.2.1

In D107304#2925220 , @joerg wrote:

> This discussion is becoming pointless.

Yes, seems we can't get agreement here, so I am planning to abandon this.

> Clang is not a 1:1 replacement for later GCC versions. Easiest example is the 
> support for `__builtin_apply` and friends.

Still we have no example when GCC x.y.z can compile something, but clang with 
-fgnuc-version=x.y.z can't process this project with some critical errors (but 
works ok with -fgnuc-version=4.2.1). But it's easy to create an example when 
-fgnuc-version=4.2.1 doesn't work (example in the description).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107304

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

[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

@teemperor is there a reason why the fixed patch never landed? This looks a 
good improvement to have.


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

https://reviews.llvm.org/D80878

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

[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Thanks!




Comment at: clang/test/CodeGen/macro-prefix-map.c:1
+// RUN: %clang_cc1 -fmacro-prefix-map=%p=/UNLIKELY/PATH -emit-llvm -o - %s | 
FileCheck %s
+

The test may be placed in CodeGenCXX/builtin-source-location.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107393

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

[PATCH] D107394: [AIX] "aligned" attribute does not decrease alignment

[Steven Wan via Phabricator via cfe-commits]

stevewan updated this revision to Diff 364186.
stevewan added a comment.

Merge tests into one file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/Layout/aix-alignof-align-and-pack-attr.cpp


Index: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | 
\
+// RUN:   FileCheck %s
+
+namespace test1 {
+struct __attribute__((__aligned__(2))) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test1{{.*}}c{{.*}} = global %"struct.test1::C" 
zeroinitializer, align 8
+} // namespace test1
+
+namespace test2 {
+struct __attribute__((__aligned__(2), packed)) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test2{{.*}}c{{.*}} = global %"struct.test2::C" 
zeroinitializer, align 2
+} // namespace test2
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2478,11 +2478,16 @@
 return ABIAlign;
 
   if (const auto *RT = T->getAs()) {
-if (TI.AlignIsRequired || RT->getDecl()->isInvalidDecl())
+const RecordDecl *RD = RT->getDecl();
+
+// When used as part of a typedef, or together with a 'packed' attribute,
+// the 'aligned' attribute can be used to decrease alignment.
+if ((TI.AlignIsRequired && T->getAs() != nullptr) ||
+RD->isInvalidDecl())
   return ABIAlign;
 
 unsigned PreferredAlign = static_cast(
-toBits(getASTRecordLayout(RT->getDecl()).PreferredAlignment));
+toBits(getASTRecordLayout(RD).PreferredAlignment));
 assert(PreferredAlign >= ABIAlign &&
"PreferredAlign should be at least as large as ABIAlign.");
 return PreferredAlign;


Index: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+namespace test1 {
+struct __attribute__((__aligned__(2))) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test1{{.*}}c{{.*}} = global %"struct.test1::C" zeroinitializer, align 8
+} // namespace test1
+
+namespace test2 {
+struct __attribute__((__aligned__(2), packed)) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test2{{.*}}c{{.*}} = global %"struct.test2::C" zeroinitializer, align 2
+} // namespace test2
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2478,11 +2478,16 @@
 return ABIAlign;
 
   if (const auto *RT = T->getAs()) {
-if (TI.AlignIsRequired || RT->getDecl()->isInvalidDecl())
+const RecordDecl *RD = RT->getDecl();
+
+// When used as part of a typedef, or together with a 'packed' attribute,
+// the 'aligned' attribute can be used to decrease alignment.
+if ((TI.AlignIsRequired && T->getAs() != nullptr) ||
+RD->isInvalidDecl())
   return ABIAlign;
 
 unsigned PreferredAlign = static_cast(
-toBits(getASTRecordLayout(RT->getDecl()).PreferredAlignment));
+toBits(getASTRecordLayout(RD).PreferredAlignment));
 assert(PreferredAlign >= ABIAlign &&
"PreferredAlign should be at least as large as ABIAlign.");
 return PreferredAlign;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for FP atomics in GFX90A

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12139
+OptimizationRemark Remark(DEBUG_TYPE, "Passed", RMW->getFunction());
+Remark << "A hardware instruction was generated";
+return Remark;

rampitec wrote:
> It was not generated. We have multiple returns below this point. Some of them 
> return None and some CmpXChg for various reasons. The request was to report 
> when we produce the instruction *if* it is unsafe, not just that we are about 
> to produce an instruction.
> 
> Then to make it useful a remark should tell what was the reason to either 
> produce an instruction or expand it. Looking at a stream of remarks in a big 
> program one would also want to understand what exactly was expanded and what 
> was left as is. A stream of messages "A hardware instruction was generated" 
> unlikely will help to understand what was done.
Will the hardware instruction be generated in the end of this function then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for FP atomics in GFX90A

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12139
+OptimizationRemark Remark(DEBUG_TYPE, "Passed", RMW->getFunction());
+Remark << "A hardware instruction was generated";
+return Remark;

gandhi21299 wrote:
> rampitec wrote:
> > It was not generated. We have multiple returns below this point. Some of 
> > them return None and some CmpXChg for various reasons. The request was to 
> > report when we produce the instruction *if* it is unsafe, not just that we 
> > are about to produce an instruction.
> > 
> > Then to make it useful a remark should tell what was the reason to either 
> > produce an instruction or expand it. Looking at a stream of remarks in a 
> > big program one would also want to understand what exactly was expanded and 
> > what was left as is. A stream of messages "A hardware instruction was 
> > generated" unlikely will help to understand what was done.
> Will the hardware instruction be generated in the end of this function then?
It will not be generated here to begin with. If the function returns None the 
atomicrmw will be just left as is and then later selected into the instruction. 
But if you read the function, it has many returns for many different reasons, 
and that is exactly what a useful remark shall report.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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