date:20220326

[PATCH] D122104: [X86][regcall] Support passing / returning structures

2022-03-26 Thread LuoYuanke via Phabricator via cfe-commits

LuoYuanke added inline comments.



Comment at: clang/include/clang/CodeGen/CGFunctionInfo.h:590
+  /// Log 2 of the maximum vector width.
+  unsigned MaxVectorWidth : 4;
+

I notice some code would indicate it is log 2 size with Log2 suffix in the 
variable name. Do you think it is more readable to add Log2 suffix?



Comment at: clang/lib/CodeGen/CGCall.cpp:5238
+  for (unsigned i = 0; i < IRCallArgs.size(); ++i)
+LargestVectorWidth = std::max(LargestVectorWidth,
+  getMaxVectorWidth(IRCallArgs[i]->getType()));

Does this also affect other calling convention besides fastcall?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:2303
   void classify(QualType T, uint64_t OffsetBase, Class &Lo, Class &Hi,
-bool isNamedArg) const;
+bool isNamedArg, bool IsRegCall = false) const;
 

Update the comments for the new parameter?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:3031
 // than eight eightbytes, ..., it has class MEMORY.
-if (Size > 512)
+if (!IsRegCall && Size > 512)
   return;

Would you add a test for non regcall? Pass 1024 bit vector parameter and check 
if it is well handled both with regcall and without regcall.
Would you add comments to depict why regcall accept the size which is more than 
512?



Comment at: clang/test/CodeGen/aarch64-neon-tbl.c:45
 
-// CHECK-LABEL: define{{.*}} <8 x i8> @test_vqtbl2_s8([2 x <16 x i8>] 
%a.coerce, <8 x i8> noundef %b) #0 {
+// CHECK-LABEL: define{{.*}} <8 x i8> @test_vqtbl2_s8([2 x <16 x i8>] 
%a.coerce, <8 x i8> noundef %b) #1 {
 // CHECK:   [[__P0_I:%.*]] = alloca %struct.int8x16x2_t, align 16

I'm curious why aarch64 test cases are affected by the patch.



Comment at: clang/test/CodeGen/regcall2.c:2
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -emit-llvm %s -o - -ffreestanding -target-feature +avx512vl 
-triple=x86_64-pc-win32 | FileCheck %s --check-prefix=Win
+// RUN: %clang_cc1 -emit-llvm %s -o - -ffreestanding -target-feature +avx512vl 
-triple=x86_64-pc-linux-gnu | FileCheck %s --check-prefix=Lin

Add test case for target that has no avx512 feature?



Comment at: clang/test/CodeGen/regcall2.c:9
+  __m512d r1[4];
+  __m512 r2[4];
+} __sVector;

May add a test case to show what's the max register we can pass with regcall.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122104

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

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

2022-03-26 Thread Vince Bridgers via Phabricator via cfe-commits

vabridgers added a comment.

Apparently, it's up to an implementation to determine the specific relations 
that can be computed between different address spaces. Perhaps a better way to 
deal with this for now, to avoid crashes, is follow the DereferenceChecker 
model. That checker punts on checking relations with any pointers that have a 
address space. If a downstream compiler wants to compute relations between 
pointers with address spaces, they are certainly free to do that - but I get 
the instinct this might not be best to push upstream :/

Ideas? Best!

Here's the snippet, and comment, from 
clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp

  ...
  112 static bool suppressReport(const Expr *E) {
  113   // Do not report dereferences on memory in non-default address spaces.
  114   return E->getType().hasAddressSpace();
  115 }
  ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122513

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

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

2022-03-26 Thread Vince Bridgers via Phabricator via cfe-commits

vabridgers added a comment.

Hi @NoQ, good question :) When I looked into the existing SimpleSValBuilder.cpp 
code, I found a few places that did width modifications so I concluded this was 
the correct and accepted way to handle this. A few notes to help decide if my 
suggestion is correct or is there's a different concrete way to move forward. 
Looks like we get address space information in the AST, and it's up to the AST 
client to do the needful (whatever that is). We do get warnings and/or errors 
in some cases, but it seems this is a case where it's implied that the "void *" 
cast is same address space as the LHS of the comparison. I'll check the 
Embedded C Spec to see what it says about this.

The Sample function looks like this:

  int fn1() {
int val = 0;
__attribute__((address_space(3))) int *dptr = val;
return dptr == (void *)0;
  }

The AST for the return statement "return dptr == (void *)0;" looks like this:

  `-ReturnStmt 0x118f2078 
`-BinaryOperator 0x118f2058  'int' '=='
  |-ImplicitCastExpr 0x118f2028  '__attribute__((address_space(3))) 
int *' 
  | `-DeclRefExpr 0x118f1fa8  '__attribute__((address_space(3))) 
int *' lvalue Var 0x118f1af0 'dptr' '__attribute__((address_space(3))) int *'
  `-ImplicitCastExpr 0x118f2040  
'__attribute__((address_space(3))) int *' 
`-CStyleCastExpr 0x118f2000  'void *' 
  `-IntegerLiteral 0x118f1fc8  'int' 0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122513

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

[PATCH] D122535: [clang-tidy] Never consider assignments as equivalent in `misc-redundant-expression` check

2022-03-26 Thread Fabian Wolff via Phabricator via cfe-commits

fwolff created this revision.
fwolff added reviewers: aaron.ballman, njames93.
fwolff added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
fwolff requested review of this revision.
Herald added a subscriber: cfe-commits.

Attempts to fix #35853 . 
This issue is about side-effectful function calls, but since almost every 
function call potentially causes side effects and would therefore prevent any 
`misc-redundant-expression` warnings, I have focused on assignments instead, 
which would also fix the example in #35853.

We already have special treatment of unary increment/decrement here 
,
 even though this can also lead to false negatives. Replacing `i++` with `i = i 
+ 1` currently changes the behavior of the `misc-redundant-expression` check, 
whereas the change proposed here prevents any warnings about redundant 
expressions if assignments are involved. This can cause false negatives, though 
(whereas the current implementation causes false positives), so let me know 
whether you think this is a sensible change to make.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122535

Files:
  clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
@@ -831,3 +831,15 @@
 };
 
 } // namespace no_crash
+
+int TestAssignSideEffect(int i) {
+  int k = i;
+
+  if ((k = k + 1) != 1 || (k = k + 1) != 2)
+return 0;
+
+  if ((k = foo(0)) != 1 || (k = foo(0)) != 2)
+return 1;
+
+  return 2;
+}
Index: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -134,6 +134,8 @@
 return cast(Left)->getOpcode() ==
cast(Right)->getOpcode();
   case Stmt::BinaryOperatorClass:
+if (cast(Left)->isAssignmentOp())
+  return false;
 return cast(Left)->getOpcode() ==
cast(Right)->getOpcode();
   case Stmt::UnaryExprOrTypeTraitExprClass:


Index: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
@@ -831,3 +831,15 @@
 };
 
 } // namespace no_crash
+
+int TestAssignSideEffect(int i) {
+  int k = i;
+
+  if ((k = k + 1) != 1 || (k = k + 1) != 2)
+return 0;
+
+  if ((k = foo(0)) != 1 || (k = foo(0)) != 2)
+return 1;
+
+  return 2;
+}
Index: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -134,6 +134,8 @@
 return cast(Left)->getOpcode() ==
cast(Right)->getOpcode();
   case Stmt::BinaryOperatorClass:
+if (cast(Left)->isAssignmentOp())
+  return false;
 return cast(Left)->getOpcode() ==
cast(Right)->getOpcode();
   case Stmt::UnaryExprOrTypeTraitExprClass:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

2022-03-26 Thread Iain Sandoe via Phabricator via cfe-commits

iains added a comment.

addendum: note we still have work to do on the module initialisers - those are 
not correct yet (so probably some nesting of modules might not work).


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

https://reviews.llvm.org/D119409

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

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

2022-03-26 Thread Iain Sandoe via Phabricator via cfe-commits

iains added a comment.

I think that this problem might well be a consequence of the bug which is fixed 
by D122413 .

We have been generating code with module internal entities (always) given the 
special ModuleInternalLinkage (which means that, although the linkage is 
formally 'internal', the entities are made global when emitted.  We should only 
be doing this for fmodules-ts, not for regular standard modules.

If you apply D122413  (which I hope to land 
soon), then I would expect that iostream should work as expected (with one 
internal instance of std::__ioinit in each TU that includes iostream).

IFF (after applying D122413  ) you add to the 
command line -fmodules-ts, then the patch here (D119409 
) would, presumably, be needed to work around 
multiple instances of the globalised std::__ioinit.


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

https://reviews.llvm.org/D119409

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

[PATCH] D114611: [AVR] Expand STDWSPQRr & STDSPQRr, approach #2

2022-03-26 Thread Patryk Wychowaniec via Phabricator via cfe-commits

Patryk27 marked 6 inline comments as done.
Patryk27 added a comment.

Thanks for the review!

For the time being, I have extracted the first set of changes into:
https://reviews.llvm.org/D122533




Comment at: llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp:1234
+
+  MachineOperand &Dst = MI.getOperand(0);
+  Register DstReg = Dst.getReg();

benshi001 wrote:
> Why we need an extra `Dst` local variable here? I did not find there is any 
> more use besides `.getReg()` .
Right, I think it must have been an oversight on my side; I have fixed it in 
the follow-up merge request (https://reviews.llvm.org/D122533).



Comment at: llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp:1246
+  // few operations
+  if (Imm >= 63) {
+if (!DstIsKill) {

benshi001 wrote:
> I suggest we make another patch for merge 
> `AVRRelaxMem::relax` into `expand`, for the 
> case `Imm >= 63`. And we select that merge patch as baseline / parent of 
> current patch.
Sure! https://reviews.llvm.org/D122533


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114611

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

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

2022-03-26 Thread Patryk Wychowaniec via Phabricator via cfe-commits

Patryk27 added a subscriber: benshi001.
Patryk27 added a comment.

cc @benshi001 (from https://reviews.llvm.org/D114611) 🙂


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122533

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

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

2022-03-26 Thread Patryk Wychowaniec via Phabricator via cfe-commits

Patryk27 updated this revision to Diff 418413.
Patryk27 retitled this revision from "[avr] Remove AVRRelaxMemOperations" to 
"[AVR] Remove AVRRelaxMemOperations".
Patryk27 added a comment.

avr -> AVR


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122533

Files:
  clang/docs/tools/clang-formatted-files.txt
  llvm/lib/Target/AVR/AVR.h
  llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
  llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp
  llvm/lib/Target/AVR/AVRTargetMachine.cpp
  llvm/lib/Target/AVR/CMakeLists.txt
  llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
  llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
  llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn
@@ -37,7 +37,6 @@
 "AVRInstrInfo.cpp",
 "AVRMCInstLower.cpp",
 "AVRRegisterInfo.cpp",
-"AVRRelaxMemOperations.cpp",
 "AVRShiftExpand.cpp",
 "AVRSubtarget.cpp",
 "AVRTargetMachine.cpp",
Index: llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
===
--- llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
+++ /dev/null
@@ -1,31 +0,0 @@
-# RUN: llc -O0 -run-pass=avr-relax-mem %s -o - | FileCheck %s
-
 |
-  target triple = "avr--"
-  define void @test() {
-  entry:
-ret void
-  }
-...
-

-name:test
-body: |
-  bb.0.entry:
-
-; CHECK-LABEL: test
-
-; We shouldn't expand things which already have 6-bit imms.
-; CHECK: STDWPtrQRr $r29r28, 63, $r1r0
-STDWPtrQRr $r29r28, 63, $r1r0
-
-; We shouldn't expand things which already have 6-bit imms.
-; CHECK-NEXT: STDWPtrQRr $r29r28, 0, $r1r0
-STDWPtrQRr $r29r28, 0, $r1r0
-
-; CHECK-NEXT: PUSHWRr $r29r28, implicit-def $sp, implicit $sp
-; CHECK-NEXT: $r29r28 = SBCIWRdK $r29r28, -64, implicit-def $sreg, implicit $sreg
-; CHECK-NEXT: STWPtrRr $r29r28, $r1r0
-; CHECK-NEXT: $r29r28 = POPWRd implicit-def $sp, implicit $sp
-STDWPtrQRr $r29r28, 64, $r1r0
-...
Index: llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
===
--- llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
+++ llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
@@ -1,4 +1,4 @@
-# RUN: llc -O0 -run-pass=avr-expand-pseudo  %s -o - | FileCheck %s
+# RUN: llc -O0 -run-pass=avr-expand-pseudo %s -o - | FileCheck %s
 
 --- |
   target triple = "avr--"
@@ -15,8 +15,52 @@
 
 ; CHECK-LABEL: test
 
-; CHECK:  STDPtrQRr $r29r28, 10, $r0
-; CHECK-NEXT: STDPtrQRr $r29r28, 11, $r1
+; Small displacement (<63):
+; CHECK:  STDPtrQRr $r29r28, 3, $r0
+; CHECK-NEXT: STDPtrQRr $r29r28, 4, $r1
+STDWPtrQRr $r29r28, 3, $r1r0
 
-STDWPtrQRr $r29r28, 10, $r1r0
+; Small displacement where the destination register is killed:
+; CHECK:  STDPtrQRr $r29r28, 3, $r0
+; CHECK-NEXT: STDPtrQRr killed $r29r28, 4, $r1
+STDWPtrQRr killed $r29r28, 3, $r1r0
+
+; Small displacement where the source register is killed:
+; CHECK:  STDPtrQRr $r29r28, 3, killed $r0
+; CHECK-NEXT: STDPtrQRr $r29r28, 4, killed $r1
+STDWPtrQRr $r29r28, 3, killed $r1r0
+
+; Small displacement, near the limit (=62):
+; CHECK:  STDPtrQRr $r29r28, 62, $r0
+; CHECK-NEXT: STDPtrQRr $r29r28, 63, $r1
+STDWPtrQRr $r29r28, 62, $r1r0
+
+; Large displacement (>=63):
+; CHECK: PUSHRr $r28, implicit-def $sp, implicit $sp
+; CHECK-NEXT: PUSHRr $r29, implicit-def $sp, implicit $sp
+; CHECK-NEXT: $r28 = SBCIRdK killed $r28, 193, implicit-def $sreg, implicit killed $sreg
+; CHECK-NEXT: $r29 = SBCIRdK killed $r29, 255, implicit-def $sreg, implicit killed $sreg
+; CHECK-NEXT: STPtrRr $r29r28, $r0
+; CHECK-NEXT: STDPtrQRr $r29r28, 1, $r1
+; CHECK-NEXT: $r29 = POPRd implicit-def $sp, implicit $sp
+; CHECK-NEXT: $r28 = POPRd implicit-def $sp, implicit $sp
+STDWPtrQRr $r29r28, 63, $r1r0
+
+; Large displacement where the destination register is killed:
+; CHECK: $r28 = SBCIRdK killed $r28, 193, implicit-def $sreg, implicit killed $sreg
+; CHECK-NEXT: $r29 = SBCIRdK killed $r29, 255, implicit-def $sreg, implicit killed $sreg
+; CHECK-NEXT: STPtrRr $r29r28, $r0
+; CHECK-NEXT: STDPtrQRr $r29r28, 1, $r1
+STDWPtrQRr killed $r29r28, 63, $r1r0
+
+; Large displacement where the source register is killed:
+; CHECK: PUSHRr $r28, implicit-def $sp, implicit $sp
+; CHECK-NEXT: PUSHRr $r29, implicit-def $sp, implicit $sp
+; CHECK-NEXT: $r28 = SBCIRdK killed $r28, 193, implicit-def $sreg, implicit killed $sreg
+; CHECK-NEXT: $r29 = SBCIRdK killed $r29, 255, implicit-def $sreg, implicit killed $sreg
+; CHECK-NEXT: STPtrRr $r29r28, killed $r0
+; CHECK-NEXT: STDPtrQRr

[PATCH] D122533: [avr] Remove AVRRelaxMemOperations

2022-03-26 Thread Patryk Wychowaniec via Phabricator via cfe-commits

Patryk27 created this revision.
Herald added subscribers: Jim, hiraditya, mgorny, dylanmckay.
Herald added a project: All.
Patryk27 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This commit contains a refactoring that merges AVRRelaxMemOperations
into AVRExpandPseudoInsts, so that we have a single place in code that
expands the STDWPtrQRr opcode.

Seizing the day, I've also fixed a couple of potential bugs with our
previous implementation (e.g. when the destination register was killed,
the previous implementation would try to `.addDef()` that killed
register, crashing LLVM in the process - that's fixed now, as proved by
the test).

In the bigger picture, this commit is the first step of fixing
https://reviews.llvm.org/D114611.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122533

Files:
  clang/docs/tools/clang-formatted-files.txt
  llvm/lib/Target/AVR/AVR.h
  llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
  llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp
  llvm/lib/Target/AVR/AVRTargetMachine.cpp
  llvm/lib/Target/AVR/CMakeLists.txt
  llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
  llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
  llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn
@@ -37,7 +37,6 @@
 "AVRInstrInfo.cpp",
 "AVRMCInstLower.cpp",
 "AVRRegisterInfo.cpp",
-"AVRRelaxMemOperations.cpp",
 "AVRShiftExpand.cpp",
 "AVRSubtarget.cpp",
 "AVRTargetMachine.cpp",
Index: llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
===
--- llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
+++ /dev/null
@@ -1,31 +0,0 @@
-# RUN: llc -O0 -run-pass=avr-relax-mem %s -o - | FileCheck %s
-
 |
-  target triple = "avr--"
-  define void @test() {
-  entry:
-ret void
-  }
-...
-

-name:test
-body: |
-  bb.0.entry:
-
-; CHECK-LABEL: test
-
-; We shouldn't expand things which already have 6-bit imms.
-; CHECK: STDWPtrQRr $r29r28, 63, $r1r0
-STDWPtrQRr $r29r28, 63, $r1r0
-
-; We shouldn't expand things which already have 6-bit imms.
-; CHECK-NEXT: STDWPtrQRr $r29r28, 0, $r1r0
-STDWPtrQRr $r29r28, 0, $r1r0
-
-; CHECK-NEXT: PUSHWRr $r29r28, implicit-def $sp, implicit $sp
-; CHECK-NEXT: $r29r28 = SBCIWRdK $r29r28, -64, implicit-def $sreg, implicit $sreg
-; CHECK-NEXT: STWPtrRr $r29r28, $r1r0
-; CHECK-NEXT: $r29r28 = POPWRd implicit-def $sp, implicit $sp
-STDWPtrQRr $r29r28, 64, $r1r0
-...
Index: llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
===
--- llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
+++ llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
@@ -1,4 +1,4 @@
-# RUN: llc -O0 -run-pass=avr-expand-pseudo  %s -o - | FileCheck %s
+# RUN: llc -O0 -run-pass=avr-expand-pseudo %s -o - | FileCheck %s
 
 --- |
   target triple = "avr--"
@@ -15,8 +15,52 @@
 
 ; CHECK-LABEL: test
 
-; CHECK:  STDPtrQRr $r29r28, 10, $r0
-; CHECK-NEXT: STDPtrQRr $r29r28, 11, $r1
+; Small displacement (<63):
+; CHECK:  STDPtrQRr $r29r28, 3, $r0
+; CHECK-NEXT: STDPtrQRr $r29r28, 4, $r1
+STDWPtrQRr $r29r28, 3, $r1r0
 
-STDWPtrQRr $r29r28, 10, $r1r0
+; Small displacement where the destination register is killed:
+; CHECK:  STDPtrQRr $r29r28, 3, $r0
+; CHECK-NEXT: STDPtrQRr killed $r29r28, 4, $r1
+STDWPtrQRr killed $r29r28, 3, $r1r0
+
+; Small displacement where the source register is killed:
+; CHECK:  STDPtrQRr $r29r28, 3, killed $r0
+; CHECK-NEXT: STDPtrQRr $r29r28, 4, killed $r1
+STDWPtrQRr $r29r28, 3, killed $r1r0
+
+; Small displacement, near the limit (=62):
+; CHECK:  STDPtrQRr $r29r28, 62, $r0
+; CHECK-NEXT: STDPtrQRr $r29r28, 63, $r1
+STDWPtrQRr $r29r28, 62, $r1r0
+
+; Large displacement (>=63):
+; CHECK: PUSHRr $r28, implicit-def $sp, implicit $sp
+; CHECK-NEXT: PUSHRr $r29, implicit-def $sp, implicit $sp
+; CHECK-NEXT: $r28 = SBCIRdK killed $r28, 193, implicit-def $sreg, implicit killed $sreg
+; CHECK-NEXT: $r29 = SBCIRdK killed $r29, 255, implicit-def $sreg, implicit killed $sreg
+; CHECK-NEXT: STPtrRr $r29r28, $r0
+; CHECK-NEXT: STDPtrQRr $r29r28, 1, $r1
+; CHECK-NEXT: $r29 = POPRd implicit-def $sp, implicit $sp
+; CHECK-NEXT: $r28 = POPRd implicit-def $sp, implicit $sp
+STDWPtrQRr $r29r28, 63, $r1r0
+
+; Large displacement where the destination register is killed:
+; CHECK: $r28 = SBCIRdK killed $r28, 193, implicit-def $sreg, implicit killed $sreg
+; CHECK-NEXT: $r29 = SBCIRdK killed $r29, 255, implicit-def $sreg, implicit killed $sreg
+; CHECK-NEXT: STPtrRr $r

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-26 Thread Hirochika Matsumoto via Phabricator via cfe-commits

hkmatsumoto added a comment.

In D122077#3405085 , @spatel wrote:

> A few changes for tests suggested inline.
>
> There might be some generalization of ctpop analysis that we can make as a 
> follow-up patch.
> For example, I was looking at a "wrong predicate" combination and noticed 
> that we miss possible optimizations like this:
> https://alive2.llvm.org/ce/z/RRk_d9

Very interesting! I'll look into it and (try to) send a follow-up PR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

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

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-26 Thread Hirochika Matsumoto via Phabricator via cfe-commits

hkmatsumoto added inline comments.



Comment at: llvm/test/Transforms/InstCombine/ispow2.ll:540
-
-define i1 @isnot_pow2_ctpop_wrong_pred1(i32 %x) {
-; CHECK-LABEL: @isnot_pow2_ctpop_wrong_pred1(

Renamed to is_pow2or0_ctpop.



Comment at: llvm/test/Transforms/InstCombine/ispow2.ll:555
-
-define i1 @isnot_pow2_ctpop_wrong_pred1_logical(i32 %x) {
-; CHECK-LABEL: @isnot_pow2_ctpop_wrong_pred1_logical(

Renamed to is_pow2or0_ctpop_logical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

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

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-26 Thread Hirochika Matsumoto via Phabricator via cfe-commits

hkmatsumoto updated this revision to Diff 418405.
hkmatsumoto added a comment.

Reflect code reviews

  

- Move all added tests to ispow2.ll from icmp-or.ll since now that they

contain tests for "and" operand

- Add tests to confirm extra uses don't affect the optimization

- Add negative tests whose order of the op/and operand being swapped

(named *_swap_cmp)

- Add negative tests for wrong predicates

- Test various wrong constants


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

Files:
  llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
  llvm/test/Transforms/InstCombine/ispow2.ll

Index: llvm/test/Transforms/InstCombine/ispow2.ll
===
--- llvm/test/Transforms/InstCombine/ispow2.ll
+++ llvm/test/Transforms/InstCombine/ispow2.ll
@@ -535,38 +535,6 @@
   ret i1 %r
 }
 
-; Negative test - wrong predicate (but this could reduce).
-
-define i1 @isnot_pow2_ctpop_wrong_pred1(i32 %x) {
-; CHECK-LABEL: @isnot_pow2_ctpop_wrong_pred1(
-; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
-;
-  %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
-  %cmp = icmp eq i32 %t0, 1
-  %iszero = icmp eq i32 %x, 0
-  %r = or i1 %iszero, %cmp
-  ret i1 %r
-}
-
-define i1 @isnot_pow2_ctpop_wrong_pred1_logical(i32 %x) {
-; CHECK-LABEL: @isnot_pow2_ctpop_wrong_pred1_logical(
-; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
-;
-  %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
-  %cmp = icmp eq i32 %t0, 1
-  %iszero = icmp eq i32 %x, 0
-  %r = select i1 %iszero, i1 true, i1 %cmp
-  ret i1 %r
-}
-
 ; Negative test - wrong predicate.
 
 define i1 @isnot_pow2_ctpop_wrong_pred2(i32 %x) {
@@ -766,3 +734,387 @@
   %r = or <2 x i1> %cmp, %iszero
   ret <2 x i1> %r
 }
+
+; (ctpop(X) == 1) || (X == 0) --> ctpop(X) u< 2
+
+define i1 @is_pow2or0_ctpop(i32 %x) {
+; CHECK-LABEL: @is_pow2or0_ctpop(
+; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
+;
+  %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
+  %cmp = icmp eq i32 %t0, 1
+  %iszero = icmp eq i32 %x, 0
+  %r = or i1 %iszero, %cmp
+  ret i1 %r
+}
+
+define i1 @is_pow2or0_ctpop_swap_cmp(i32 %x) {
+; CHECK-LABEL: @is_pow2or0_ctpop_swap_cmp(
+; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
+;
+  %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
+  %cmp = icmp eq i32 %t0, 1
+  %iszero = icmp eq i32 %x, 0
+  %r = or i1 %cmp, %iszero
+  ret i1 %r
+}
+
+define i1 @is_pow2or0_ctpop_logical(i32 %x) {
+; CHECK-LABEL: @is_pow2or0_ctpop_logical(
+; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
+;
+  %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
+  %cmp = icmp eq i32 %t0, 1
+  %iszero = icmp eq i32 %x, 0
+  %r = select i1 %iszero, i1 true, i1 %cmp
+  ret i1 %r
+}
+
+define <2 x i1> @is_pow2or0_ctpop_commute_vec(<2 x i8> %x) {
+; CHECK-LABEL: @is_pow2or0_ctpop_commute_vec(
+; CHECK-NEXT:[[T0:%.*]] = tail call <2 x i8> @llvm.ctpop.v2i8(<2 x i8> [[X:%.*]])
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult <2 x i8> [[T0]], 
+; CHECK-NEXT:ret <2 x i1> [[TMP1]]
+;
+  %t0 = tail call <2 x i8> @llvm.ctpop.v2i8(<2 x i8> %x)
+  %cmp = icmp eq <2 x i8> %t0, 
+  %iszero = icmp eq <2 x i8> %x, 
+  %r = or <2 x i1> %iszero, %cmp
+  ret <2 x i1> %r
+}
+
+; Extra uses don't change the fold.
+
+define i1 @is_pow2or0_ctpop_extra_uses(i32 %x) {
+; CHECK-LABEL: @is_pow2or0_ctpop_extra_uses(
+; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:call void @use(i32 [[T0]])
+; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
+; CHECK-NEXT:call void @use_i1(i1 [[CMP]])
+; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
+; CHECK-NEXT:call void @use_i1(i1 [[ISZERO]])
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
+;
+  %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
+  call void @use(i32 %t0)
+  %cmp = icmp eq i32 %t0, 1
+  call void @use_i1(i1 %cmp)
+  %iszero = icmp eq i32 %x, 0
+  call void @use_i1(i1 %iszero)
+  %r = or i1 %iszero, %cmp
+  ret i1 %r
+}
+
+define i1 @is_pow2or0_ctpop_logical_extra_uses(i32 %x) {

[clang] f884622 - [C++20][Modules][HU 3/5] Emit module macros for header units.

2022-03-26 Thread Iain Sandoe via cfe-commits


Author: Iain Sandoe
Date: 2022-03-26T16:30:40Z
New Revision: f8846229c41f3e4aede3bd06921772a209b4993a

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

LOG: [C++20][Modules][HU 3/5] Emit module macros for header units.

For header units we build the top level module directly from the header
that it represents and macros defined in this TU need to be emitted (when
such a definition is live at the end of the TU).

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

Added: 
clang/test/Modules/cxx20-hu-04.cpp

Modified: 
clang/include/clang/Basic/Module.h
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Module.h 
b/clang/include/clang/Basic/Module.h
index 9752ff61e4a27..4500c78ec6824 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -527,6 +527,9 @@ class Module {
Kind == ModulePartitionImplementation;
   }
 
+  /// Is this module a header unit.
+  bool isHeaderUnit() const { return Kind == ModuleHeaderUnit; }
+
   /// Get the primary module interface name from a partition.
   StringRef getPrimaryModuleInterfaceName() const {
 if (isModulePartition()) {

diff  --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index e455e4d4d96a5..6cbf3dd20017a 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -18,6 +18,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/Module.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"

diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index c06cbb251c408..90e7fb1714f40 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2352,13 +2352,22 @@ void ASTWriter::WritePreprocessor(const Preprocessor 
&PP, bool IsModule) {
 uint64_t StartOffset = Stream.GetCurrentBitNo() - MacroOffsetsBase;
 assert((StartOffset >> 32) == 0 && "Macro identifiers offset too large");
 
-// Emit the macro directives in reverse source order.
-for (; MD; MD = MD->getPrevious()) {
-  // Once we hit an ignored macro, we're done: the rest of the chain
-  // will all be ignored macros.
-  if (shouldIgnoreMacro(MD, IsModule, PP))
-break;
-
+// Write out any exported module macros.
+bool EmittedModuleMacros = false;
+// C+=20 Header Units are compiled module interfaces, but they preserve
+// macros that are live (i.e. have a defined value) at the end of the
+// compilation.  So when writing a header unit, we preserve only the final
+// value of each macro (and discard any that are undefined).  Header units
+// do not have sub-modules (although they might import other header units).
+// PCH files, conversely, retain the history of each macro's define/undef
+// and of leaf macros in sub modules.
+if (IsModule && WritingModule->isHeaderUnit()) {
+  // This is for the main TU when it is a C++20 header unit.
+  // We preserve the final state of defined macros, and we do not emit ones
+  // that are undefined.
+  if (!MD || shouldIgnoreMacro(MD, IsModule, PP) ||
+  MD->getKind() == MacroDirective::MD_Undefine)
+continue;
   AddSourceLocation(MD->getLocation(), Record);
   Record.push_back(MD->getKind());
   if (auto *DefMD = dyn_cast(MD)) {
@@ -2366,35 +2375,51 @@ void ASTWriter::WritePreprocessor(const Preprocessor 
&PP, bool IsModule) {
   } else if (auto *VisMD = dyn_cast(MD)) {
 Record.push_back(VisMD->isPublic());
   }
-}
+  ModuleMacroRecord.push_back(getSubmoduleID(WritingModule));
+  ModuleMacroRecord.push_back(getMacroRef(MD->getMacroInfo(), Name));
+  Stream.EmitRecord(PP_MODULE_MACRO, ModuleMacroRecord);
+  ModuleMacroRecord.clear();
+  EmittedModuleMacros = true;
+} else {
+  // Emit the macro directives in reverse source order.
+  for (; MD; MD = MD->getPrevious()) {
+// Once we hit an ignored macro, we're done: the rest of the chain
+// will all be ignored macros.
+if (shouldIgnoreMacro(MD, IsModule, PP))
+  break;
+AddSourceLocation(MD->getLocation(), Record);
+Record.push_back(MD->getKind());
+if (auto *DefMD = dyn_cast(MD)) {
+  Record.push_back(getMacroRef(DefMD->getInfo(), Name));
+} else if (auto *VisMD = dyn_cast(MD)) {
+  Record.push_back(VisMD->isPublic());
+}
+  }
 
-// Write out any exported module macros.
-bool EmittedModu

[PATCH] D121097: [C++20][Modules][HU 3/5] Emit module macros for header units.

2022-03-26 Thread Iain Sandoe via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf8846229c41f: [C++20][Modules][HU 3/5] Emit module macros 
for header units. (authored by iains).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121097

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/cxx20-hu-04.cpp

Index: clang/test/Modules/cxx20-hu-04.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-hu-04.cpp
@@ -0,0 +1,105 @@
+// Test macro preservation in C++20 Header Units.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header hu-01.h \
+// RUN: -o hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info hu-01.pcm | \
+// RUN: FileCheck --check-prefix=CHECK-HU %s -DTDIR=%t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header hu-02.h \
+// RUN: -DFOO -fmodule-file=hu-01.pcm -o hu-02.pcm  -Rmodule-import 2>&1 | \
+// RUN: FileCheck --check-prefix=CHECK-IMP %s -DTDIR=%t
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info hu-02.pcm | \
+// RUN: FileCheck --check-prefix=CHECK-HU2 %s -DTDIR=%t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface importer-01.cpp \
+// RUN:  -fmodule-file=hu-02.pcm -o B.pcm -DTDIR=%t -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface importer-02.cpp \
+// RUN:  -fmodule-file=hu-02.pcm -o C.pcm -DTDIR=%t -Rmodule-import 2>&1 | \
+// RUN:  FileCheck --check-prefix=CHECK-IMP-HU2 %s -DTDIR=%t
+
+//--- hu-01.h
+#ifndef __GUARD
+#define __GUARD
+
+int baz(int);
+#define FORTYTWO 42
+
+#define SHOULD_NOT_BE_DEFINED -1
+#undef SHOULD_NOT_BE_DEFINED
+
+#endif // __GUARD
+// expected-no-diagnostics
+
+// CHECK-HU:  == C++20 Module structure ==
+// CHECK-HU-NEXT:  Header Unit './hu-01.h' is the Primary Module at index #1
+
+//--- hu-02.h
+export import "hu-01.h";
+#if !defined(FORTYTWO) || FORTYTWO != 42
+#error FORTYTWO missing in hu-02
+#endif
+
+#ifndef __GUARD
+#error __GUARD missing in hu-02
+#endif
+
+#ifdef SHOULD_NOT_BE_DEFINED
+#error SHOULD_NOT_BE_DEFINED is visible
+#endif
+
+#define KAP 6174
+
+#ifdef FOO
+#define FOO_BRANCH(X) (X) + 1
+inline int foo(int x) {
+  if (x == FORTYTWO)
+return FOO_BRANCH(x);
+  return FORTYTWO;
+}
+#else
+#define BAR_BRANCH(X) (X) + 2
+inline int bar(int x) {
+  if (x == FORTYTWO)
+return BAR_BRANCH(x);
+  return FORTYTWO;
+}
+#endif
+
+// CHECK-IMP: remark: importing module './hu-01.h' from 'hu-01.pcm'
+// CHECK-HU2:  == C++20 Module structure ==
+// CHECK-HU2-NEXT:  Header Unit './hu-02.h' is the Primary Module at index #2
+// CHECK-HU2-NEXT:   Exports:
+// CHECK-HU2-NEXT:Header Unit './hu-01.h' is at index #1
+// expected-no-diagnostics
+
+//--- importer-01.cpp
+export module B;
+import "hu-02.h";
+
+int success(int x) {
+  return foo(FORTYTWO + x + KAP);
+}
+
+int fail(int x) {
+  return bar(FORTYTWO + x + KAP); // expected-error {{use of undeclared identifier 'bar'}}
+  // expected-note@* {{'baz' declared here}}
+}
+
+//--- importer-02.cpp
+export module C;
+import "hu-02.h";
+
+int success(int x) {
+  return foo(FORTYTWO + x + KAP);
+}
+
+// CHECK-IMP-HU2: remark: importing module './hu-02.h' from 'hu-02.pcm'
+// CHECK-IMP-HU2: remark: importing module './hu-01.h' into './hu-02.h' from '[[TDIR]]{{[/\\]}}hu-01.pcm'
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2352,13 +2352,22 @@
 uint64_t StartOffset = Stream.GetCurrentBitNo() - MacroOffsetsBase;
 assert((StartOffset >> 32) == 0 && "Macro identifiers offset too large");
 
-// Emit the macro directives in reverse source order.
-for (; MD; MD = MD->getPrevious()) {
-  // Once we hit an ignored macro, we're done: the rest of the chain
-  // will all be ignored macros.
-  if (shouldIgnoreMacro(MD, IsModule, PP))
-break;
-
+// Write out any exported module macros.
+bool EmittedModuleMacros = false;
+// C+=20 Header Units are compiled module interfaces, but they preserve
+// macros that are live (i.e. have a defined value) at the end of the
+// compilation.  So when writing a header unit, we preserve only the final
+// value of each macro (and discard any that are undefined).  Header units
+// do not have sub-modules (although they might import other header units).
+// PCH files, conversely, retain the history of each macro's define/undef
+// and of leaf macros in sub modules.
+if (IsModule && WritingModule->isHeaderUnit()) {
+  // This is for the main TU when it is a C++20 header unit.
+  // We preserve the final state of defined macros, and we do not emit

[PATCH] D122529: [ASTMatchers] Output currently matching node on crash

2022-03-26 Thread Nathan James via Phabricator via cfe-commits

njames93 created this revision.
njames93 added reviewers: klimek, aaron.ballman, LegalizeAdulthood.
Herald added a project: All.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Extend D120185  to also log the node being 
matched on in case of a crash.
This can help if a matcher is causing a crash or there are not enough 
interesting nodes bound.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122529

Files:
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -39,10 +39,24 @@
 // FIXME: Figure out why back traces aren't being generated on clang builds on
 // windows.
 #if ENABLE_BACKTRACES && (!defined(_MSC_VER) || !defined(__clang__))
+
+AST_MATCHER(Decl, causeCrash) {
+  LLVM_BUILTIN_TRAP;
+  return true;
+}
+
+TEST(MatcherCrashDeathTest, CrashOnMatcherDump) {
+  llvm::EnablePrettyStackTrace();
+  auto Matcher = testing::ContainsRegex(
+  "ASTMatcher: Matching '' against:\n\tFunctionDecl foo : "
+  "");
+  ASSERT_DEATH(matches("void foo();", functionDecl(causeCrash())), Matcher);
+}
+
 template 
 static void crashTestNodeDump(MatcherT Matcher,
   ArrayRef MatchedNodes,
-  StringRef Code) {
+  StringRef Against, StringRef Code) {
   llvm::EnablePrettyStackTrace();
   MatchFinder Finder;
 
@@ -58,7 +72,9 @@
 ASSERT_DEATH(tooling::runToolOnCode(
  newFrontendActionFactory(&Finder)->create(), Code),
  testing::HasSubstr(
- "ASTMatcher: Processing 'CrashTester'\nNo bound nodes"));
+ ("ASTMatcher: Processing 'CrashTester' against:\n\t" +
+  Against + "\nNo bound nodes")
+ .str()));
   } else {
 std::vector>>
@@ -69,7 +85,9 @@
 }
 auto CrashMatcher = testing::AllOf(
 testing::HasSubstr(
-"ASTMatcher: Processing 'CrashTester'\n--- Bound Nodes Begin ---"),
+("ASTMatcher: Processing 'CrashTester' against:\n\t" + Against +
+ "\n--- Bound Nodes Begin ---")
+.str()),
 testing::HasSubstr("--- Bound Nodes End ---"),
 testing::AllOfArray(Matchers));
 
@@ -79,7 +97,8 @@
   }
 }
 TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
-  crashTestNodeDump(forStmt(), {}, "void foo() { for(;;); }");
+  crashTestNodeDump(forStmt(), {}, "ForStmt : ",
+"void foo() { for(;;); }");
   crashTestNodeDump(
   forStmt(hasLoopInit(declStmt(hasSingleDecl(
varDecl(hasType(qualType().bind("QT")),
@@ -94,6 +113,7 @@
"IL - { IntegerLiteral :  }", "QT - { QualType : int }",
"T - { BuiltinType : int }",
"VD - { VarDecl I :  }"},
+  "ForStmt : ",
   R"cpp(
   void foo() {
 for (int I = 0; I < 5; ++I) {
@@ -106,12 +126,14 @@
   {"Unnamed - { CXXRecordDecl (anonymous) :  }",
"Op+ - { CXXMethodDecl (anonymous struct)::operator+ :  }"},
+  "CXXRecordDecl (anonymous) : ",
   "struct { int operator+(int) const; } Unnamed;");
   crashTestNodeDump(
   cxxRecordDecl(hasMethod(cxxConstructorDecl(isDefaulted()).bind("Ctor")),
 hasMethod(cxxDestructorDecl(isDefaulted()).bind("Dtor"))),
   {"Ctor - { CXXConstructorDecl Foo::Foo :  }",
"Dtor - { CXXDestructorDecl Foo::~Foo :  }"},
+  "CXXRecordDecl Foo : ",
   "struct Foo { Foo() = default; ~Foo() = default; };");
 }
 #endif // ENABLE_BACKTRACES
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -761,53 +761,166 @@
 D);
   }
 
+private:
+  bool TraversingASTNodeNotSpelledInSource = false;
+  bool TraversingASTNodeNotAsIs = false;
+  bool TraversingASTChildrenNotSpelledInSource = false;
+
+  class CurMatchData {
+  public:
+CurMatchData() = default;
+
+template 
+void SetCallbackAndRawNode(const MatchCallback *CB, const NodeType &N) {
+  assertEmpty();
+  Callback = CB;
+  MatchingNode = &N;
+}
+
+const MatchCallback *getCallback() const { return Callback; }
+
+void SetBoundNodes(const BoundNodes &BN) {
+  assertHoldsState();
+  BNodes = &BN;
+}
+
+void clearBoundNodes() {
+  assertHoldsState();
+  BNodes = nullptr;
+}
+
+template  const T *getNode() const {
+  assertHoldsState();
+  return MatchingNode.dyn_cast();
+}
+
+const BoundNodes *getBoundNodes() const {
+  assertHoldsState();
+

[PATCH] D122528: [clang][ASTImporter] Not using consumeError at failed import of in-class initializer.

2022-03-26 Thread Balázs Kéri via Phabricator via cfe-commits

balazske added a reviewer: martong.
balazske added a comment.
Herald added a subscriber: rnkovacs.

I wanted to remove this one instance of `consumeError` because the change in 
D122525 , to ensure that `consumeError` is 
called really only from `ImportDeclContext` (and other places where it is not 
relevant).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122528

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

[PATCH] D122528: [clang][ASTImporter] Not using consumeError at failed import of in-class initializer.

2022-03-26 Thread Balázs Kéri via Phabricator via cfe-commits

balazske created this revision.
Herald added subscribers: steakhal, martong, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The error can be returned from the function, the problem written in comment 
before
does not exist. The same is done already in ASTImporter at various import 
failures.

After a declaration is created in an `ASTNodeImporter` import function
with `GetImportedOrCreateDecl`, that function registers it with
`MapImported`. At many places import errors can happen after this
and the error is returned. The same can be done in the place where
the in-class initializer is imported.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122528

Files:
  clang/lib/AST/ASTImporter.cpp


Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3652,13 +3652,8 @@
   !FoundField->getInClassInitializer()) {
 if (ExpectedExpr ToInitializerOrErr = import(FromInitializer))
   FoundField->setInClassInitializer(*ToInitializerOrErr);
-else {
-  // We can't return error here,
-  // since we already mapped D as imported.
-  // FIXME: warning message?
-  consumeError(ToInitializerOrErr.takeError());
-  return FoundField;
-}
+else
+  return ToInitializerOrErr.takeError();
   }
 }
 return FoundField;


Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3652,13 +3652,8 @@
   !FoundField->getInClassInitializer()) {
 if (ExpectedExpr ToInitializerOrErr = import(FromInitializer))
   FoundField->setInClassInitializer(*ToInitializerOrErr);
-else {
-  // We can't return error here,
-  // since we already mapped D as imported.
-  // FIXME: warning message?
-  consumeError(ToInitializerOrErr.takeError());
-  return FoundField;
-}
+else
+  return ToInitializerOrErr.takeError();
   }
 }
 return FoundField;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121459: [analyzer] Remove HasAlphaDocumentation tablegen enum value

2022-03-26 Thread Gabor Marton via Phabricator via cfe-commits

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

Hahh, indeed. IMHO, it does not make sense to write an "alpha" documentation, 
either write it properly or do not write anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121459

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

[PATCH] D122244: [analyzer] Turn missing tablegen doc entry of a checker into fatal error

2022-03-26 Thread Gabor Marton via Phabricator via cfe-commits

martong accepted this revision.
martong added a comment.

Good.


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

https://reviews.llvm.org/D122244

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

[PATCH] D122243: [analyzer][NFC] Introduce the checker package separator character

2022-03-26 Thread Gabor Marton via Phabricator via cfe-commits

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122243

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

[PATCH] D122524: [clang][AVR] Eliminate link warnings when '-S' is specified

2022-03-26 Thread Ben Shi via Phabricator via cfe-commits

benshi001 added a comment.

1. If '-S'/'-c' is specified, do not generate link warnings;
  - User should be clear that he does not want link, so there is no link 
warning.

2. If '-S'/'-c' is not specified, '-mmcu' is specified and there is valid GCC 
installation, do not generate link warnings;
  - This is the normal case, link should be succesful.

3. If '-S'/'-c' is not specified, and '-mmcu' is not specified, genereate link 
warnings;
  - Since no MCU is specified, we should warn the user that no device library 
is linked.

4. If '-S'/'-c' is not specified, and there is no valid avr-GCC installation, 
genereate link warnings.
  - Since there is no avr-ld installed, we should warn the user that there is 
no link happens. That may changes if we choose to use lld in the future.


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

https://reviews.llvm.org/D122524

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

[PATCH] D122496: [C11] Correct the resulting type for an assignment expression

2022-03-26 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman closed this revision.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Thanks for the reviews, I've commit in bfa2f25d350c1015b74b8a14684e68efa6500bbc 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122496

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

[clang] bfa2f25 - [C11] Correct the resulting type for an assignment expression

2022-03-26 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2022-03-26T08:03:11-04:00
New Revision: bfa2f25d350c1015b74b8a14684e68efa6500bbc

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

LOG: [C11] Correct the resulting type for an assignment expression

In C, assignment expressions result in an rvalue whose type is the type
of the lhs of the assignment after it undergoes lvalue to rvalue
conversion. lvalue to rvalue conversion in C strips all qualifiers
including _Atomic.

We used getUnqualifiedType() which does not strip the _Atomic qualifier
when we should have used getAtomicUnqualifiedType(). This corrects the
usage and adds some comments to getUnqualifiedType() to make it more
clear that it does not strip _Atomic and that's on purpose (see C11
6.2.5p27).

This addresses Issue 48742.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/Type.h
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/atomic-expr.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9e7958918b2dc..3f37dfe6027a4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -82,7 +82,9 @@ Bug Fixes
   alias, target) identifier instead of only processing one such ``#pragma 
weak``
   per identifier.
   Fixes `Issue 28985 `_.
-
+- Assignment expressions in C11 and later mode now properly strip the _Atomic
+  qualifier when determining the type of the assignment expression. Fixes
+  `Issue 48742 `_.
 - Unevaluated lambdas in dependant contexts no longer result in clang crashing.
   This fixes Issues `50376 
`_,
   `51414 `_,

diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index bc66ffbf735e2..56869c17bd5cf 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -931,6 +931,10 @@ class QualType {
   /// The resulting type might still be qualified if it's sugar for an array
   /// type.  To strip qualifiers even from within a sugared array type, use
   /// ASTContext::getUnqualifiedArrayType.
+  ///
+  /// Note: In C, the _Atomic qualifier is special (see C2x 6.2.5p29 for
+  /// details), and it is not stripped by this function. Use
+  /// getAtomicUnqualifiedType() to strip qualifiers including _Atomic.
   inline QualType getUnqualifiedType() const;
 
   /// Retrieve the unqualified variant of the given type, removing as little

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 1547a34b5c730..c5ede6eb3f908 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -13637,15 +13637,15 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, 
ExprResult &RHS,
 }
   }
 
-  // C99 6.5.16p3: The type of an assignment expression is the type of the
-  // left operand unless the left operand has qualified type, in which case
-  // it is the unqualified version of the type of the left operand.
-  // C99 6.5.16.1p2: In simple assignment, the value of the right operand
-  // is converted to the type of the assignment expression (above).
+  // C11 6.5.16p3: The type of an assignment expression is the type of the
+  // left operand would have after lvalue conversion.
+  // C11 6.3.2.1p2: ...this is called lvalue conversion. If the lvalue has
+  // qualified type, the value has the unqualified version of the type of the
+  // lvalue; additionally, if the lvalue has atomic type, the value has the
+  // non-atomic version of the type of the lvalue.
   // C++ 5.17p1: the type of the assignment expression is that of its left
   // operand.
-  return (getLangOpts().CPlusPlus
-  ? LHSType : LHSType.getUnqualifiedType());
+  return getLangOpts().CPlusPlus ? LHSType : 
LHSType.getAtomicUnqualifiedType();
 }
 
 // Only ignore explicit casts to void.

diff  --git a/clang/test/Sema/atomic-expr.c b/clang/test/Sema/atomic-expr.c
index 3afcebd0d6cce..097ed4fd39ee4 100644
--- a/clang/test/Sema/atomic-expr.c
+++ b/clang/test/Sema/atomic-expr.c
@@ -61,3 +61,17 @@ int func_13 (int x, unsigned y) {
 int func_14 (void) {
   return data1 == 0;
 }
+
+void func_15(void) {
+  // Ensure that the result of an assignment expression properly strips the
+  // _Atomic qualifier; Issue 48742.
+  _Atomic int x;
+  int y = (x = 2);
+  int z = (int)(x = 2);
+  y = (x = 2);
+  z = (int)(x = 2);
+  y = (x += 2);
+
+  _Static_assert(__builtin_types_compatible_p(__typeof__(x = 2), int), 
"incorrect");
+  _Static_assert(__builtin_types_compatible_p(__typeof__(x += 2), int), 
"incorrect");
+}



___
cfe-commits mailing list
cfe-commits@l

[PATCH] D122496: [C11] Correct the resulting type for an assignment expression

2022-03-26 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:13647
   // operand.
-  return (getLangOpts().CPlusPlus
-  ? LHSType : LHSType.getUnqualifiedType());
+  return getLangOpts().CPlusPlus ? LHSType : 
LHSType.getAtomicUnqualifiedType();
 }

cor3ntin wrote:
> I wonder if we want another comment here to make it extra clear that atomic 
> should be stripped?
I've updated the standard wording comments above to make this more clear; good 
call!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122496

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

[PATCH] D122423: [Clang][doc] Fix __builtin_assume wording.

2022-03-26 Thread Mark de Wever 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 rGc3b672a34cde: [Clang][doc] Fix __builtin_assume wording. 
(authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122423

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2294,7 +2294,8 @@
 The boolean argument to this function is defined to be true. The optimizer may
 analyze the form of the expression provided as the argument and deduce from
 that information used to optimize the program. If the condition is violated
-during execution, the behavior is undefined. The argument itself is 
+during execution, the behavior is undefined. The argument itself is never
+evaluated, so any side effects of the expression will be discarded.
 
 Query for this feature with ``__has_builtin(__builtin_assume)``.
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2294,7 +2294,8 @@
 The boolean argument to this function is defined to be true. The optimizer may
 analyze the form of the expression provided as the argument and deduce from
 that information used to optimize the program. If the condition is violated
-during execution, the behavior is undefined. The argument itself is 
+during execution, the behavior is undefined. The argument itself is never
+evaluated, so any side effects of the expression will be discarded.
 
 Query for this feature with ``__has_builtin(__builtin_assume)``.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c3b672a - [Clang][doc] Fix __builtin_assume wording.

2022-03-26 Thread Mark de Wever via cfe-commits


Author: Mark de Wever
Date: 2022-03-26T13:02:40+01:00
New Revision: c3b672a34cdef38002016ab15454c2df432e48c8

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

LOG: [Clang][doc] Fix __builtin_assume wording.

D117296 removed wording for __builtin_assume, D120205 restored the
wording, but the last sentence was only partly restored. This restores
the rest of the last sentence.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/docs/LanguageExtensions.rst

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 05d8d53d44c1c..45fb8efaf8b01 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -2294,7 +2294,8 @@ invariant that is defined to be true.
 The boolean argument to this function is defined to be true. The optimizer may
 analyze the form of the expression provided as the argument and deduce from
 that information used to optimize the program. If the condition is violated
-during execution, the behavior is undefined. The argument itself is 
+during execution, the behavior is undefined. The argument itself is never
+evaluated, so any side effects of the expression will be discarded.
 
 Query for this feature with ``__has_builtin(__builtin_assume)``.
 



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

[PATCH] D122423: [Clang][doc] Fix __builtin_assume wording.

2022-03-26 Thread Mark de Wever via Phabricator via cfe-commits

Mordante added a comment.

In D122423#3406381 , @aaron.ballman 
wrote:

> LGTM, a second time. :-D Sorry for missing that the first time.

No problem, thanks for the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122423

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

[PATCH] D122524: [clang][AVR] Eliminate link warnings when '-S' is specified

2022-03-26 Thread Ben Shi via Phabricator via cfe-commits

benshi001 updated this revision to Diff 418387.
benshi001 added a comment.

1. If '-S'/'-c' is specified, do not generate link warnings;
2. If '-S'/'-c' is not specified, '-mmcu' is specified and there is valid GCC 
installation, do not generate link warnings;
3. If '-S'/'-c' is not specified, and '-mmcu' is not specified, genereate link 
warnings;
4. If '-S'/'-c' is not specified, and there is no valid GCC installation, 
genereate link warnings.


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

https://reviews.llvm.org/D122524

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -35,3 +35,30 @@
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdinc | FileCheck --check-prefix=NOSTDINC %s
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdlibinc | FileCheck --check-prefix=NOSTDINC %s
 // NOSTDINC-NOT: "-internal-isystem" {{".*avr/include"}}
+
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 
-mmcu=atmega328 2>&1 | FileCheck --check-prefix=CHECK5 %s
+// CHECK5-NOT: warning: no target microcontroller specified on command line, 
cannot link standard libraries
+// CHECK5-NOT: warning: no avr-gcc installation can be found on the system, 
cannot link standard libraries
+// CHECK5-NOT: warning: standard library not linked and so no interrupt vector 
table or compiler runtime routines will be linked
+
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/ -mmcu=atmega328 2>&1 | 
FileCheck --check-prefix=CHECK6 %s
+// CHECK6-NOT: warning: no target microcontroller specified on command line, 
cannot link standard libraries
+// CHECK6: warning: no avr-gcc installation can be found on the system, cannot 
link standard libraries
+// CHECK6: warning: standard library not linked and so no interrupt vector 
table or compiler runtime routines will be linked
+
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 | 
FileCheck --check-prefix=CHECK7 %s
+// CHECK7: warning: no target microcontroller specified on command line, 
cannot link standard libraries
+// CHECK7-NOT: warning: no avr-gcc installation can be found on the system, 
cannot link standard libraries
+// CHECK7: warning: standard library not linked and so no interrupt vector 
table or compiler runtime routines will be linked
+
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree -c 2>&1 
| FileCheck --check-prefix=CHECK8 %s
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree -S 2>&1 
| FileCheck --check-prefix=CHECK8 %s
+// CHECK8: warning: no target microcontroller specified on command line, 
cannot link standard libraries
+// CHECK8-NOT: warning: no avr-gcc installation can be found on the system, 
cannot link standard libraries
+// CHECK8-NOT: warning: standard library not linked and so no interrupt vector 
table or compiler runtime routines will be linked
+
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 
-mmcu=atmega328 -c 2>&1 | FileCheck --check-prefix=CHECK9 %s
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 
-mmcu=atmega328 -S 2>&1 | FileCheck --check-prefix=CHECK9 %s
+// CHECK9-NOT: warning: no target microcontroller specified on command line, 
cannot link standard libraries
+// CHECK9-NOT: warning: no avr-gcc installation can be found on the system, 
cannot link standard libraries
+// CHECK9-NOT: warning: standard library not linked and so no interrupt vector 
table or compiler runtime routines will be linked
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -369,16 +369,18 @@
 : Generic_ELF(D, Triple, Args), LinkStdlib(false) {
   GCCInstallation.init(Triple, Args);
 
+  std::string CPU = getCPUName(D, Args, Triple);
+  if (CPU.empty())
+D.Diag(diag::warn_drv_avr_mcu_not_specified);
+
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs) &&
+  !Args.hasArg(options::OPT_S) &&
   !Args.hasArg(options::OPT_c /* does not apply when not linking */)) {
-std::string CPU = getCPUName(D, Args, Triple);
 
-if (CPU.empty()) {
-  // We cannot link any standard libraries without an MCU specified.
-  D.Diag(diag::warn_drv_avr_mcu_not_specified);
-} else {
+// We cannot link any standard libraries without an MCU specified.
+if (!CPU.empty()) {
   Optional FamilyName = GetMCUFamilyName(CPU);
   Optional AVRLibcRoot = findAVRLibcInstallation();
 


Index: clang/test/Driver/avr-toolchain.c
==

[PATCH] D122425: Pass -disable-llvm-passes to avoid running llvm passes

2022-03-26 Thread Evgeny Shulgin via Phabricator via cfe-commits

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

Hi! I don't actually know much about llvm passes, but I see that the LLVM IR 
output is much more understandable now, thanks! It will look nicer when I 
rebase my patch D119792  on top of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122425

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

[PATCH] D121096: [C++20][Modules][HU 2/5] Support searching Header Units in user or system search paths.

2022-03-26 Thread Iain Sandoe via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0687578728ea: [C++20][Modules][HU 2/5] Support searching 
Header Units in user or system… (authored by iains).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121096

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/test/Modules/cxx20-hu-02.cpp
  clang/test/Modules/cxx20-hu-03.cpp
  clang/test/Modules/cxx20-hu-bad-input.cpp

Index: clang/test/Modules/cxx20-hu-bad-input.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-hu-bad-input.cpp
@@ -0,0 +1,19 @@
+// Test generation and import of simple C++20 Header Units.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: not %clang_cc1 -std=c++20 -emit-header-unit \
+// RUN:  -xc++-header-unit-header hu-01.hh \
+// RUN:  -xc++-header-unit-header hu-02.hh \
+// RUN:  -o hu-01.pcm -verify  2>&1 | FileCheck %s
+
+// CHECK: (frontend): multiple inputs are not valid for header units (first extra 'hu-02.hh')
+
+//--- hu-01.hh
+int foo(int);
+
+//--- hu-02.hh
+int bar(int);
Index: clang/test/Modules/cxx20-hu-03.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-hu-03.cpp
@@ -0,0 +1,57 @@
+// Test check that processing headers as C++20 units allows #pragma once.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header hu-01.h \
+// RUN: -Werror -o hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header hu-02.h \
+// RUN: -fmodule-file=%t/hu-01.pcm -o hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only imports-01.cpp \
+// RUN: -fmodule-file=%t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only imports-02.cpp \
+// RUN: -fmodule-file=%t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only imports-03.cpp \
+// RUN: -fmodule-file=%t/hu-02.pcm
+
+//--- hu-01.h
+#pragma once
+struct HU {
+  int a;
+};
+// expected-no-diagnostics
+
+//--- hu-02.h
+export import "hu-01.h";
+// expected-no-diagnostics
+
+//--- imports-01.cpp
+import "hu-01.h";
+
+HU foo(int x) {
+  return {x};
+}
+// expected-no-diagnostics
+
+//--- imports-02.cpp
+import "hu-02.h";
+
+HU foo(int x) {
+  return {x};
+}
+// expected-no-diagnostics
+
+//--- imports-03.cpp
+import "hu-01.h";
+import "hu-02.h";
+
+HU foo(int x) {
+  return {x};
+}
+// expected-no-diagnostics
Index: clang/test/Modules/cxx20-hu-02.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-hu-02.cpp
@@ -0,0 +1,77 @@
+// Test generation and import of user and system C++20 Header Units.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// check user path
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -I user \
+// RUN: -xc++-user-header hu-01.h -o hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info hu-01.pcm | \
+// RUN: FileCheck --check-prefix=CHECK-HU %s -DTDIR=%t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface imp-hu-01.cpp \
+// RUN:  -I user -fmodule-file=hu-01.pcm -o B.pcm -Rmodule-import \
+// RUN: 2>&1  | FileCheck --check-prefix=CHECK-IMP %s -DTDIR=%t
+
+// check system path
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -isystem system \
+// RUN: -xc++-system-header hu-02.h -o hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info hu-02.pcm | \
+// RUN: FileCheck --check-prefix=CHECK-HU2 %s -DTDIR=%t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface imp-hu-02.cpp \
+// RUN:  -isystem system -fmodule-file=hu-02.pcm -o C.pcm \
+// RUN: -Rmodule-import 2>&1 | \
+// RUN: FileCheck --check-prefix=CHECK-SYS-IMP %s -DTDIR=%t
+
+// check absolute path
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit  \
+// RUN: -xc++-header-unit-header %t/hu-03.h -o hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info hu-03.pcm | \
+// RUN: FileCheck --check-prefix=CHECK-HU3 %s -DTDIR=%t
+
+//--- user/hu-01.h
+int foo(int);
+
+// CHECK-HU:  == C++20 Module structure ==
+// CHECK-HU-NEXT:  Header Unit 'user{{[/\\]}}hu-01.h' is the Primary Module at index #1
+
+//--- imp-hu-01.cpp
+export module B;
+import "hu-01.h";
+
+int bar(int x) {
+  return foo(x);
+}
+// CHECK-IMP: remark: importing module 'user{{[/\\]}}hu-01.h' from 'hu-01.pcm'
+// expected-no-diagnostics
+
+//--- system/hu-02.h
+int baz(int);
+
+// CHECK-HU2:  == C++20 Module structure ==
+// CHECK-HU2-NEXT:  Header Unit 'system{{[/\\]}}hu-02.h' is the Primary Module at index #1
+
+//--- imp-hu-02.cpp
+module;
+import ;
+
+export module C;
+
+int bar(int x) {
+  return baz(x);
+}
+// CHECK-SYS-IMP: remark: importing module 'syst

[clang] 0687578 - [C++20][Modules][HU 2/5] Support searching Header Units in user or system search paths.

2022-03-26 Thread Iain Sandoe via cfe-commits


Author: Iain Sandoe
Date: 2022-03-26T10:17:17Z
New Revision: 0687578728ea1985cbab0de14d4eeb4e89cdf210

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

LOG: [C++20][Modules][HU 2/5] Support searching Header Units in user or system 
search paths.

This is support for the user-facing options to create importable header units
from headers in the user or system search paths (or to be given an absolute 
path).

This means that an incomplete header path will be passed by the driver and the
lookup carried out using the search paths present when the front end is run.

To support this, we introduce file fypes for 
c++-{user,system,header-unit}-header.
These terms are the same as the ones used by GCC, to minimise the differences 
for
tooling (and users).

The preprocessor checks for headers before issuing a warning for
"#pragma once" in a header build.  We ensure that the importable header units
are recognised as headers in order to avoid such warnings.

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

Added: 
clang/test/Modules/cxx20-hu-02.cpp
clang/test/Modules/cxx20-hu-03.cpp
clang/test/Modules/cxx20-hu-bad-input.cpp

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Frontend/FrontendOptions.h
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Frontend/FrontendAction.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 59862555cc8f6..63913b933c8fb 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -266,6 +266,8 @@ def err_drv_cc_print_options_failure : Error<
 def err_drv_lto_without_lld : Error<"LTO requires -fuse-ld=lld">;
 def err_drv_preamble_format : Error<
 "incorrect format for -preamble-bytes=N,END">;
+def err_drv_header_unit_extra_inputs : Error<
+"multiple inputs are not valid for header units (first extra '%0')">;
 def warn_invalid_ios_deployment_target : Warning<
   "invalid iOS deployment version '%0', iOS 10 is the maximum deployment "
   "target for 32-bit targets">, InGroup,

diff  --git a/clang/include/clang/Frontend/FrontendOptions.h 
b/clang/include/clang/Frontend/FrontendOptions.h
index 160dc71505e2f..ff5a9c5c77f4d 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -153,6 +153,8 @@ class InputKind {
   Language Lang;
   unsigned Fmt : 3;
   unsigned Preprocessed : 1;
+  unsigned HeaderUnit : 3;
+  unsigned IsHeader : 1;
 
 public:
   /// The input file format.
@@ -162,13 +164,29 @@ class InputKind {
 Precompiled
   };
 
+  // If we are building a header unit, what kind it is; this affects whether
+  // we look for the file in the user or system include search paths before
+  // flagging a missing input.
+  enum HeaderUnitKind {
+HeaderUnit_None,
+HeaderUnit_User,
+HeaderUnit_System,
+HeaderUnit_Abs
+  };
+
   constexpr InputKind(Language L = Language::Unknown, Format F = Source,
-  bool PP = false)
-  : Lang(L), Fmt(F), Preprocessed(PP) {}
+  bool PP = false, HeaderUnitKind HU = HeaderUnit_None,
+  bool HD = false)
+  : Lang(L), Fmt(F), Preprocessed(PP), HeaderUnit(HU), IsHeader(HD) {}
 
   Language getLanguage() const { return static_cast(Lang); }
   Format getFormat() const { return static_cast(Fmt); }
+  HeaderUnitKind getHeaderUnitKind() const {
+return static_cast(HeaderUnit);
+  }
   bool isPreprocessed() const { return Preprocessed; }
+  bool isHeader() const { return IsHeader; }
+  bool isHeaderUnit() const { return HeaderUnit != HeaderUnit_None; }
 
   /// Is the input kind fully-unknown?
   bool isUnknown() const { return Lang == Language::Unknown && Fmt == Source; }
@@ -179,11 +197,23 @@ class InputKind {
   }
 
   InputKind getPreprocessed() const {
-return InputKind(getLanguage(), getFormat(), true);
+return InputKind(getLanguage(), getFormat(), true, getHeaderUnitKind(),
+ isHeader());
+  }
+
+  InputKind getHeader() const {
+return InputKind(getLanguage(), getFormat(), isPreprocessed(),
+ getHeaderUnitKind(), true);
+  }
+
+  InputKind withHeaderUnit(HeaderUnitKind HU) const {
+return InputKind(getLanguage(), getFormat(), isPreprocessed(), HU,
+ isHeader());
   }
 
   InputKind withFormat(Format F) const {
-return InputKind(getLanguage(), F, isPreprocessed());
+return InputKind(getLanguage(), F, isPreprocessed(), getHeaderUnitKind(),
+ isHeader());
   }
 };
 
@@ -218,6 +248,10 @@ class FrontendInputFile {
   bool isFile() const { return !isBuffer(); }
   bool isBuff

[PATCH] D122525: [clang][ASTImporter] Fix an import error handling related bug.

2022-03-26 Thread Balázs Kéri via Phabricator via cfe-commits

balazske created this revision.
Herald added subscribers: steakhal, martong, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This bug can cause that more import errors are generated than necessary
and many objects fail to import. Chance of an invalid AST after these
imports increases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122525

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5806,6 +5806,53 @@
   EXPECT_FALSE(ImportedF);
 }
 
+TEST_P(ErrorHandlingTest, DoNotInheritErrorFromNonDependentChild) {
+  // Declarations should not inherit an import error from a child object
+  // if the declaration has no direct dependence to such a child.
+  // For example a namespace should not get import error if one of the
+  // declarations inside it fails to import.
+  // There was a special case in error handling (when "import path circles" are
+  // encountered) when this property was not held. This case is provoked by the
+  // following code.
+  constexpr auto ToTUCode = R"(
+  namespace ns {
+struct Err {
+  char A;
+};
+  }
+  )";
+  constexpr auto FromTUCode = R"(
+  namespace ns {
+struct A {
+  using U = struct Err;
+};
+  }
+  namespace ns {
+struct Err {}; // ODR violation
+void f(A) {}
+  }
+  )";
+
+  Decl *ToTU = getToTuDecl(ToTUCode, Lang_CXX11);
+  static_cast(ToTU);
+  Decl *FromTU = getTuDecl(FromTUCode, Lang_CXX11);
+  auto *FromA = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("A"), hasDefinition()));
+  ASSERT_TRUE(FromA);
+  auto *ImportedA = Import(FromA, Lang_CXX11);
+  // 'A' can not be imported: ODR error at 'Err'
+  EXPECT_FALSE(ImportedA);
+  // When import of 'A' failed there was a "saved import path circle" that
+  // contained namespace 'ns' (A - U - Err - ns - f - A). This should not mean
+  // that every object in this path fails to import.
+
+  Decl *FromNS = FirstDeclMatcher().match(
+  FromTU, namespaceDecl(hasName("ns")));
+  EXPECT_TRUE(FromNS);
+  auto *ImportedNS = Import(FromNS, Lang_CXX11);
+  EXPECT_TRUE(ImportedNS);
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) {
   Decl *FromTU = getTuDecl(
   R"(
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8776,8 +8776,24 @@
 
 // Set the error for all nodes which have been created before we
 // recognized the error.
-for (const auto &Path : SavedImportPaths[FromD])
+for (const auto &Path : SavedImportPaths[FromD]) {
+  Decl *PrevFromDi = FromD;
   for (Decl *FromDi : Path) {
+// Begin and end of the path equals 'FromD', skip it.
+if (FromDi == FromD)
+  continue;
+// We should not set import error on a node that "consumes" the error
+// during import of its children (failing import of the child does not
+// cause failure of the parent). See ImportDeclContext and uses of
+// 'consumeError'.
+// The import path contains child nodes first.
+// (Parent-child relationship is used here in sense of import
+// dependency.)
+if (!isa(FromDi))
+  if (auto *FromDiDC = dyn_cast(FromDi))
+if (FromDiDC->containsDecl(PrevFromDi))
+  break; // Skip every following node in the path.
+PrevFromDi = FromDi;
 setImportDeclError(FromDi, ErrOut);
 //FIXME Should we remove these Decls from ImportedDecls?
 // Set the error for the mapped to Decl, which is in the "to" context.
@@ -8787,6 +8803,7 @@
   // FIXME Should we remove these Decls from the LookupTable,
   // and from ImportedFromDecls?
   }
+}
 SavedImportPaths.erase(FromD);
 
 // Do not return ToDOrErr, error was taken out of it.


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5806,6 +5806,53 @@
   EXPECT_FALSE(ImportedF);
 }
 
+TEST_P(ErrorHandlingTest, DoNotInheritErrorFromNonDependentChild) {
+  // Declarations should not inherit an import error from a child object
+  // if the declaration has no direct dependence to such a child.
+  // For example a namespace should not get import error if one of the
+  // declarations inside it fails to import.
+  // There was a special case in error handling (when "import path c

[PATCH] D121302: [HIP] Fix -fno-gpu-sanitize

2022-03-26 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/HIPAMD.cpp:165
   // Diagnose unsupported sanitizer options only once.
+  if (!Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize))
+return;

Note: the third argument of hasFlag defaults to true before I removed it in 
522712e2d241ea33575a9c7a60ad582634f04f0d. I suspect it might not be what you 
intended. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121302

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

[clang] c37accf - [Option] Avoid using the default argument for the 3-argument hasFlag. NFC

2022-03-26 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2022-03-26T00:57:06-07:00
New Revision: c37accf0a207a09b80c1109eb7a114876c45f959

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

LOG: [Option] Avoid using the default argument for the 3-argument hasFlag. NFC

The default argument true is error-prone: I think many would think the
default is false.

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/lib/Driver/ToolChains/AMDGPU.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/HIPAMD.cpp
clang/lib/Frontend/CompilerInvocation.cpp
lld/COFF/Driver.cpp
llvm/tools/llvm-objcopy/ObjcopyOptions.cpp

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 31ef07cd5acc6..dd89121e671e5 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -3024,7 +3024,7 @@ class OffloadingActionBuilder final {
   if (Args.hasArg(options::OPT_gpu_bundle_output,
   options::OPT_no_gpu_bundle_output))
 BundleOutput = Args.hasFlag(options::OPT_gpu_bundle_output,
-options::OPT_no_gpu_bundle_output);
+options::OPT_no_gpu_bundle_output, true);
 }
 
 bool canUseBundlerUnbundler() const override { return true; }

diff  --git a/clang/lib/Driver/SanitizerArgs.cpp 
b/clang/lib/Driver/SanitizerArgs.cpp
index 440f29142ab29..598fe9a3bf0fd 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1016,8 +1016,8 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const 
llvm::opt::ArgList &Args,
   // AMDGPU sanitizer support is experimental and controlled by -fgpu-sanitize.
   if (TC.getTriple().isNVPTX() ||
   (TC.getTriple().isAMDGPU() &&
-   !Args.hasFlag(options::OPT_fgpu_sanitize,
- options::OPT_fno_gpu_sanitize)))
+   !Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize,
+ true)))
 return;
 
   // Translate available CoverageFeatures to corresponding clang-cc1 flags.

diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp 
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 8df86ca4f4784..123d92664495e 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -947,7 +947,7 @@ ROCMToolChain::getCommonDeviceLibNames(const 
llvm::opt::ArgList &DriverArgs,
 options::OPT_fno_fast_math, false);
   bool CorrectSqrt = DriverArgs.hasFlag(
   options::OPT_fhip_fp32_correctly_rounded_divide_sqrt,
-  options::OPT_fno_hip_fp32_correctly_rounded_divide_sqrt);
+  options::OPT_fno_hip_fp32_correctly_rounded_divide_sqrt, true);
   bool Wave64 = isWave64(DriverArgs, Kind);
 
   return RocmInstallation.getCommonBitcodeLibs(

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 7c7f9254b34da..6c4bc86787290 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3905,7 +3905,7 @@ static void RenderDiagnosticsOptions(const Driver &D, 
const ArgList &Args,
 
   // -fdiagnostics-fixit-info is default, only pass non-default.
   if (!Args.hasFlag(options::OPT_fdiagnostics_fixit_info,
-options::OPT_fno_diagnostics_fixit_info))
+options::OPT_fno_diagnostics_fixit_info, true))
 CmdArgs.push_back("-fno-diagnostics-fixit-info");
 
   // Enable -fdiagnostics-show-option by default.
@@ -3974,7 +3974,7 @@ static void RenderDiagnosticsOptions(const Driver &D, 
const ArgList &Args,
 CmdArgs.push_back("-fansi-escape-codes");
 
   if (!Args.hasFlag(options::OPT_fshow_source_location,
-options::OPT_fno_show_source_location))
+options::OPT_fno_show_source_location, true))
 CmdArgs.push_back("-fno-show-source-location");
 
   if (Args.hasArg(options::OPT_fdiagnostics_absolute_paths))
@@ -3985,7 +3985,7 @@ static void RenderDiagnosticsOptions(const Driver &D, 
const ArgList &Args,
 CmdArgs.push_back("-fno-show-column");
 
   if (!Args.hasFlag(options::OPT_fspell_checking,
-options::OPT_fno_spell_checking))
+options::OPT_fno_spell_checking, true))
 CmdArgs.push_back("-fno-spell-checking");
 }
 
@@ -4780,7 +4780,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 
 // Render the CodeGen options that need to be passed.
 if (!Args.hasFlag(options::OPT_foptimize_sibling_calls,
-  options::OPT_fno_optimize_sibling_calls))
+  options::OPT_fno_optimize_sibling_calls, true))
   CmdArgs.push_back("-mdisable-tail-calls");
 
 RenderFloatingPoi

[PATCH] D122524: [clang][AVR] Eliminate link warnings when '-S' is specified

2022-03-26 Thread Ben Shi via Phabricator via cfe-commits

benshi001 created this revision.
benshi001 added reviewers: aykevl, MaskRay.
Herald added subscribers: StephenFan, Jim, dylanmckay.
Herald added a project: All.
benshi001 requested review of this revision.
Herald added subscribers: cfe-commits, jacquesguan.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122524

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -35,3 +35,10 @@
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdinc | FileCheck --check-prefix=NOSTDINC %s
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdlibinc | FileCheck --check-prefix=NOSTDINC %s
 // NOSTDINC-NOT: "-internal-isystem" {{".*avr/include"}}
+
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 | 
FileCheck --check-prefix=LINK %s
+// LINK: warning: standard library not linked and so no interrupt vector table 
or compiler runtime routines will be linked
+
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree -S 2>&1 
| FileCheck --check-prefix=NOLINK %s
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree -c 2>&1 
| FileCheck --check-prefix=NOLINK %s
+// NOLINK-NOT: warning: standard library not linked and so no interrupt vector 
table or compiler runtime routines will be linked
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -372,6 +372,7 @@
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs) &&
+  !Args.hasArg(options::OPT_S) &&
   !Args.hasArg(options::OPT_c /* does not apply when not linking */)) {
 std::string CPU = getCPUName(D, Args, Triple);
 


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -35,3 +35,10 @@
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdinc | FileCheck --check-prefix=NOSTDINC %s
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdlibinc | FileCheck --check-prefix=NOSTDINC %s
 // NOSTDINC-NOT: "-internal-isystem" {{".*avr/include"}}
+
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 | FileCheck --check-prefix=LINK %s
+// LINK: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
+
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree -S 2>&1 | FileCheck --check-prefix=NOLINK %s
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree -c 2>&1 | FileCheck --check-prefix=NOLINK %s
+// NOLINK-NOT: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -372,6 +372,7 @@
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs) &&
+  !Args.hasArg(options::OPT_S) &&
   !Args.hasArg(options::OPT_c /* does not apply when not linking */)) {
 std::string CPU = getCPUName(D, Args, Triple);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118095: [clang][AVR] Reject non assembly source files for the avr1 family

2022-03-26 Thread Ben Shi via Phabricator via cfe-commits

benshi001 added a comment.

In D118095#3408037 , @aykevl wrote:

> @MaskRay it was my suggestion to move this from the toolchain specific file 
> to the generic file, because it makes the implementation much simpler. See my 
> comment D117423#3251110  for 
> details.
>
> In D118095#3282039 , @MaskRay wrote:
>
>> Rejecting some -mmcu= for C source files looks quite dubious. Does it really 
>> help users?
>
> For context: the avr1 family isn't supported by avr-gcc either. It's a old 
> and rather limited subset of the AVR instruction set. I assume it's going to 
> be rather difficult (and not worth the trouble) to write a C compiler for it, 
> as it doesn't even have a fully functional stack. From Wikipedia 
> :
>
>> The AVR1 subset was not popular and no new models have been introduced since 
>> 2000. It omits all RAM except for the 32 registers mapped at address 0–31 
>> and the I/O ports at addresses 32–95. The stack is replaced by a 3-level 
>> hardware stack, and the PUSH and POP instructions are deleted. All 16-bit 
>> operations are deleted, as are IJMP, ICALL, and all load and store 
>> addressing modes except indirect via Z.
>
> So I think the idea is to disallow this family so that users won't 
> accidentally try to use a C compiler for these chips. However, writing 
> assembly might still make sense.

My opinion is: we indeed need to be compatible with avr-gcc (accept assembly 
but reject c progarms), but we do it in AVR specific files and let common code 
clean.


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

https://reviews.llvm.org/D118095

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

37 matches

Mail list logo