[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

I measured a stage 2 `-DLLVM_TARGETS_TO_BUILD=X86 -DCMAKE_BUILD_TYPE=Release 
-DLLVM_BUILD_INSTRUMENTED_COVERAGE=on` build on Linux x86-64.

  % ls -1 /tmp/out/s3-custom/**/*.o(.) | wc -l
  2174
  
  % stat -c %s /tmp/out/s3-custom/**/*.o(.) | awk '{s+=$1}END{print s}'
  6174683864
  % stat -c %s /tmp/out/s2-custom/**/*.o(.) | awk '{s+=$1}END{print s}' 


 
  5992574864
  
  % stat -c %s /tmp/out/s3-custom/bin/clang-13
  683184016
  % stat -c %s /tmp/out/s2-custom/bin/clang-13 
  617970256

For ELF the object file size decrease is 3%.  The non-stripped clang size 
decrease is 10%.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103372

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

[PATCH] D103720: [clang] NFC: Rename rvalue to prvalue

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 350006.
mizvekov added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103720

Files:
  clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/ExprObjC.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/Initialization.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConcepts.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ExprObjC.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/Analysis/ThreadSafety.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaFixItUtils.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaPseudoObject.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/AST/ast-dump-decl-json.c
  clang/test/AST/ast-dump-decl-json.m
  clang/test/AST/ast-dump-expr-json.c
  clang/test/AST/ast-dump-expr-json.cpp
  clang/test/AST/ast-dump-expr-json.m
  clang/test/AST/ast-dump-funcs-json.cpp
  clang/test/AST/ast-dump-if-json.cpp
  clang/test/AST/ast-dump-objc-arc-json.m
  clang/test/AST/ast-dump-record-definition-data-json.cpp
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-stmt-json.c
  clang/test/AST/ast-dump-stmt-json.cpp
  clang/test/AST/ast-dump-stmt-json.m
  clang/test/AST/ast-dump-template-decls-json.cpp
  clang/test/AST/ast-dump-temporaries-json.cpp
  clang/test/AST/ast-dump-types-errors-json.cpp
  clang/test/AST/multistep-explicit-cast-json.c
  clang/test/AST/multistep-explicit-cast-json.cpp

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

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: clang/docs/UsersManual.rst:1484
+   Where unsafe floating point optimizations are enabled, this option
+   determines whether the optimizer honors parentheses when floating-point
+   expressions are evaluated.  If unsafe floating point optimizations are

aaron.ballman wrote:
> mibintc wrote:
> > aaron.ballman wrote:
> > > We may need to expound on what "honor parentheses" means. The summary for 
> > > the patch mentions `(a + b) + c`, but does this also mean `(x + y) * z`  
> > > could be interpreted as `x + (y * z)`? What about for non-arithmetic 
> > > expressions involving parentheses, like `(float1 == float2 && float1 == 
> > > float3) || blah()`?
> > Thanks for your review. Just a quick first response, What do you mean "(x + 
> > y) * z could be reinterpreted as x + (y * z)" -- not following you here? 
> "whether the optimizer honors the parentheses" reads to me like `(x + y) * z` 
> could either honor or not honor the parentheses, so that could either 
> evaluate as `(x + y) * z` or `x + (y * z)` depending on whether the parens 
> are honored or not.
I would prefer to start with a description of the intended behavior from first 
principles. The logic here should not be tied to fast-mode as it also overlaps 
a lot with the formulation of fused multiply-add expressions and certainly 
should have specified behavior on the interaction. I'm also not sure about what 
you mean with value-safe FP modes, IIRC the C/C++ standard explicitly leave it 
open in which order floating point expressions of the same operand priority are 
evaluated.

I'm also a bit puzzled why __arithmethic_fence should not be considered a 
constant expression?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100118

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

[PATCH] D103108: [CUDA][HIP] Promote const variables to constant

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added inline comments.



Comment at: clang/test/CodeGenCUDA/device-use-host-var.cu:68
+// NEG-NOT: @_ZL13var_host_only
+// NEG-NOT: external
 

tra wrote:
> yaxunl wrote:
> > sbc100 wrote:
> > > This seems to break if the pathname where the llvm checkout lives 
> > > contains the word "external"
> > > 
> > > https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket.appspot.com/8845343598940893760/+/steps/LLVM_regression/0/stdout
> > > 
> > > ```
> > > --
> > > /b/s/w/ir/cache/builder/emscripten-releases/llvm-project/clang/test/CodeGenCUDA/device-use-host-var.cu:68:13:
> > >  error: NEG-NOT: excluded string found in input
> > > // NEG-NOT: external
> > > ^
> > > :69:78: note: found here
> > > !1 = !{!"clang version 13.0.0 
> > > (/b/s/w/ir/cache/git/chromium.googlesource.com-external-github.com-llvm-llvm--project
> > >  db3e4faa4d2cadf204e67f42bccd98957496a87a)"}
> > >   
> > >^~~~
> > > ```
> > > 
> > > Unfortunate choice of directory names I know .. but probably worth fixing.
> > thanks. this has been fixed by https://reviews.llvm.org/D103658
> > 
> This should already be fixed by D103658
Indeed!  Our latest build is now passing.  Sorry for the noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103108

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

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:147
+class ExpressionHandler;
+class StoreHandler;
+

vsavchenko wrote:
> xazax.hun wrote:
> > During path sensitive analysis we already have a callback for stores. We 
> > kind of replicating this logic for bug paths. 
> > So my questions are:
> > * Do we expect to have additional information here that were not available 
> > during the analysis earlier?
> > * Do we want to make this as similar to the forward analysis part as 
> > possible for developer familiarity?
> Oof, I actually think that this one is trickier.  If the checker modeled some 
> operation (most likely `evalCall`), it can bind a value to the region by 
> itself.  And this handler triggers for such events as well because it simply 
> finds the place where the switch was flipped and the region changed its 
> binding.
> If the checker modeled some operation (most likely evalCall), it can bind a 
> value to the region by itself. And this handler triggers for such events as 
> well because it simply finds the place where the switch was flipped and the 
> region changed its binding.

This is such a great point, I'd like that to be captured as a comment 
somewhere. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103605

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

[PATCH] D103108: [CUDA][HIP] Promote const variables to constant

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/test/CodeGenCUDA/device-use-host-var.cu:68
+// NEG-NOT: @_ZL13var_host_only
+// NEG-NOT: external
 

yaxunl wrote:
> sbc100 wrote:
> > This seems to break if the pathname where the llvm checkout lives contains 
> > the word "external"
> > 
> > https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket.appspot.com/8845343598940893760/+/steps/LLVM_regression/0/stdout
> > 
> > ```
> > --
> > /b/s/w/ir/cache/builder/emscripten-releases/llvm-project/clang/test/CodeGenCUDA/device-use-host-var.cu:68:13:
> >  error: NEG-NOT: excluded string found in input
> > // NEG-NOT: external
> > ^
> > :69:78: note: found here
> > !1 = !{!"clang version 13.0.0 
> > (/b/s/w/ir/cache/git/chromium.googlesource.com-external-github.com-llvm-llvm--project
> >  db3e4faa4d2cadf204e67f42bccd98957496a87a)"}
> > 
> >  ^~~~
> > ```
> > 
> > Unfortunate choice of directory names I know .. but probably worth fixing.
> thanks. this has been fixed by https://reviews.llvm.org/D103658
> 
This should already be fixed by D103658


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103108

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

[PATCH] D103108: [CUDA][HIP] Promote const variables to constant

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGenCUDA/device-use-host-var.cu:68
+// NEG-NOT: @_ZL13var_host_only
+// NEG-NOT: external
 

sbc100 wrote:
> This seems to break if the pathname where the llvm checkout lives contains 
> the word "external"
> 
> https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket.appspot.com/8845343598940893760/+/steps/LLVM_regression/0/stdout
> 
> ```
> --
> /b/s/w/ir/cache/builder/emscripten-releases/llvm-project/clang/test/CodeGenCUDA/device-use-host-var.cu:68:13:
>  error: NEG-NOT: excluded string found in input
> // NEG-NOT: external
> ^
> :69:78: note: found here
> !1 = !{!"clang version 13.0.0 
> (/b/s/w/ir/cache/git/chromium.googlesource.com-external-github.com-llvm-llvm--project
>  db3e4faa4d2cadf204e67f42bccd98957496a87a)"}
>  
> ^~~~
> ```
> 
> Unfortunate choice of directory names I know .. but probably worth fixing.
thanks. this has been fixed by https://reviews.llvm.org/D103658



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103108

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

[PATCH] D103108: [CUDA][HIP] Promote const variables to constant

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added inline comments.



Comment at: clang/test/CodeGenCUDA/device-use-host-var.cu:68
+// NEG-NOT: @_ZL13var_host_only
+// NEG-NOT: external
 

This seems to break if the pathname where the llvm checkout lives contains the 
word "external"

https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket.appspot.com/8845343598940893760/+/steps/LLVM_regression/0/stdout

```
--
/b/s/w/ir/cache/builder/emscripten-releases/llvm-project/clang/test/CodeGenCUDA/device-use-host-var.cu:68:13:
 error: NEG-NOT: excluded string found in input
// NEG-NOT: external
^
:69:78: note: found here
!1 = !{!"clang version 13.0.0 
(/b/s/w/ir/cache/git/chromium.googlesource.com-external-github.com-llvm-llvm--project
 db3e4faa4d2cadf204e67f42bccd98957496a87a)"}
 
^~~~
```

Unfortunate choice of directory names I know .. but probably worth fixing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103108

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

[PATCH] D103722: [clang] NFC: test for undefined behaviour in RawComment::getFormattedText()

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

Thanks for adding more tests! LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103722

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

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

In D103440#2799629 , @xazax.hun wrote:

> I was wondering, if we could try something new with the tests. To increase 
> our confidence that the expected behavior is correct, how about including a 
> Z3 proof with each of the test cases?

We are looking forward to design a unit-test framework for the solver which can 
compact the test cases and make them much more manageable (unlike 
`constant-folding.c`). Perhaps, we can incorporate the Z3 proves in that 
framework, corresponding to test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103440

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

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103605#2799853 , @NoQ wrote:

> In D103605#2798581 , @vsavchenko 
> wrote:
>
>> In D103605#2798171 , @NoQ wrote:
>>
>>> - Trackers are attached to bug reports. A single bug report may have 
>>> multiple trackers.
>>>   - Why not have only one tracker per report?
>>
>> That's a good question, we can actually even make it a part of `BugReport` 
>> for that matter.  I can though think of one exotic case, where you wait till 
>> you encounter certain weird condition, where you need to spawn a very 
>> specific customized tracker only starting from that point.
>
> Ok, let's take a simpler example:
>
>   int foo(int D) {
> int A = 1;
> int B = 1;
> int C = B - A;
> return D / C; // division by zero
>   }
>
> And suppose we want to track `A` and `B` so that to explain why we think that 
> `C` is zero. What are pros and cons of:
> (1) tracking `A` and `B` within the same tracker that already helped us reach 
> `C`?
> (2) terminating the current tracker as it reaches `C` and spawning two 
> separate trackers for `A` and `B`?
>
>>> - The termination condition is as usual: no further visitors attached.
>>>   - It sounds like this patchset is orthogonal to the problem of detecting 
>>> the tracking termination point(?) It does make it easier to implement 
>>> customized reactions to such termination though.
>>
>> Actually, tracker dies when this happens.  But hey, can you provide a use 
>> case when the checker really needs to know when this happens?
>
> Inlined defensive check suppressions are a major example. Basically null 
> dereference warnings are suppressed based on properties of the program point 
> at which the tracker terminates. If we know when this happens, we can 
> refactor inlined defensive check suppressions into a checker specific handler.

It seems to me that it is a hard concept to wrap your mind around.  And even if 
I would tell someone how we suppress null pointer inline checks, I wouldn't use 
"when the tracker stops" as a description of the event.  It is very vague and 
tightly coupled with the implementation of tracker stopping, and it is not 
connected semantically to the problem we are trying to solve.  Correct me if 
I'm wrong, but better description would be track when we constrained the 
pointer value to null, and if it in the child stack frame, drop it.
So, maybe instead of very hard to understand "when the tracker stops", we can 
use "when the constraint changes" or "when the constraint became this".  This 
is more similar to store handlers in their nature, we find a pair of states 
where some condition stops holding (man, that can have a generic 
implementation!), describe it, and call the handler.

>>> - However, instead of adding notes directly, they simply invoke `track()`. 
>>> All note-attaching work is moved into additional default handlers.
>>
>> No, they call `track` where they used to call `trackExpressionValue` or add 
>> `FindLastStoreBRVisitor`.
>> Right now, only stores received their special customization and can produce 
>> notes based on tracker customization.
>
> What's the ultimate plan though, do we want to fully move all notes into 
> handlers?

There was no such plan, honestly.  I had one problem in mind: it is hard to 
detect stores yourself, `FindLastStoreBRVisitor` is very tightly coupled with 
`trackExpressionValue`, and you either put your code in it, or... forget about. 
Yep, there is no other option!
And it's not unheard of that checkers want to know how the value got into this 
very region, and it is not unheard of wanting to change messages on those.  
This component seems to be generic enough in identifying stores and classifying 
them, but horrible in the way that it gives you a fixed set of notes: leave it 
or take it.
I don't instantly see how other visitors from that set have necessities to be 
customized, but we already found one about changing constraints to be useful.

>>> - Let's consider an example. Suppose you're implementing `evalCall` for 
>>> `llvm::dyn_cast`. You could have your expression handler detect a modeled 
>>> `dyn_cast` call and implement its `handle()` method to emit a 
>>> checker-specific note and start tracking the argument.
>>
>> So, in here it would look like this: If I want to support `dyn_cast` for 
>> tracking, I will add extra `ExpressionHandler` that calls `track` on the 
>> expression inside of `llvm::dyn_cast`.  Additionally, if you want to change 
>> the note's message for situations like `a = dyn_cast(b)`, you can add a 
>> `StoreHandler` that will produce a note like "a is casted from b".
>>
>>> - With this implementation such note will only be emitted if the value is 
>>> tracked back to `dyn_cast`. Not every modeled `dyn_cast` will receive a 
>>> note but only the one that, say, caused a null dereference on its return 
>>> value when it faile

[PATCH] D103722: [clang] NFC: test for undefined behaviour in RawComment::getFormattedText()

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin created this revision.
DmitryPolukhin added reviewers: teemperor, obruns, bruno.
DmitryPolukhin requested review of this revision.
Herald added a project: clang.

This diff adds testcase for the issue fixed in https://reviews.llvm.org/D77468
but regression test was not added in the diff. On Clang 9 it caused
crash in cland during code completion.

Test Plan: check-clang-unit


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103722

Files:
  clang/unittests/AST/CommentTextTest.cpp


Index: clang/unittests/AST/CommentTextTest.cpp
===
--- clang/unittests/AST/CommentTextTest.cpp
+++ clang/unittests/AST/CommentTextTest.cpp
@@ -124,4 +124,11 @@
   // clang-format on
 }
 
+TEST_F(CommentTextTest, EmptyFormattedText) {
+  // Test that empty formatted text doesn't cause crash.
+  auto ExpectedOutput = "";
+  auto Formatted = formatComment("//!<");
+  EXPECT_EQ(ExpectedOutput, Formatted);
+}
+
 } // namespace clang


Index: clang/unittests/AST/CommentTextTest.cpp
===
--- clang/unittests/AST/CommentTextTest.cpp
+++ clang/unittests/AST/CommentTextTest.cpp
@@ -124,4 +124,11 @@
   // clang-format on
 }
 
+TEST_F(CommentTextTest, EmptyFormattedText) {
+  // Test that empty formatted text doesn't cause crash.
+  auto ExpectedOutput = "";
+  auto Formatted = formatComment("//!<");
+  EXPECT_EQ(ExpectedOutput, Formatted);
+}
+
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97699: [analyzer] Add InvalidPtrChecker

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:180
+// Note: This pointer has type 'const MemRegion *'
+REGISTER_TRAIT_WITH_PROGRAMSTATE(EnvPtrRegion, const void *)
+

balazske wrote:
> Is it not possible to use `const MemRegion *` then?
The trait stuff is only specialized for void pointers, instead of for any 
pointer type.
TBH I don't know why is that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97699

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

[clang] 33ba8bd - [Windows SEH]: Fix -O2 crash for Windows -EHa

[Ten Tzen via cfe-commits]

Author: Ten Tzen
Date: 2021-06-04T14:07:44-07:00
New Revision: 33ba8bd2c942062731a17e1b864b5953e3d79f1a

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

LOG: [Windows SEH]: Fix -O2 crash for Windows -EHa

This patch fixes a Windows -EHa crash induced by previous commit 
797ad701522988e212495285dade8efac41a24d4.
The crash was caused by "LifetimeMarker" scope (with option -O2) that should 
not be considered as SEH Scope.

This change also turns off -fasync-exceptions by default under -EHa option for 
now.

Differential Revision: https://reviews.llvm.org/D103664#2799944

Added: 


Modified: 
clang/lib/CodeGen/CGCleanup.cpp
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index b439204cf047..b9364fcd2231 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -195,7 +195,7 @@ void *EHScopeStack::pushCleanup(CleanupKind Kind, size_t 
Size) {
 Scope->setLifetimeMarker();
 
   // With Windows -EHa, Invoke llvm.seh.scope.begin() for EHCleanup
-  if (CGF->getLangOpts().EHAsynch && IsEHCleanup &&
+  if (CGF->getLangOpts().EHAsynch && IsEHCleanup && !IsLifetimeMarker &&
   CGF->getTarget().getCXXABI().isMicrosoft())
 CGF->EmitSehCppScopeBegin();
 

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index ee40df35b850..a0e1208fd709 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7151,8 +7151,6 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID 
InputType,
 if (types::isCXX(InputType))
   CmdArgs.push_back("-fcxx-exceptions");
 CmdArgs.push_back("-fexceptions");
-if (EH.Asynch)
-  CmdArgs.push_back("-fasync-exceptions");
   }
   if (types::isCXX(InputType) && EH.Synch && EH.NoUnwindC)
 CmdArgs.push_back("-fexternc-nounwind");



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

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

[Ten Tzen via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG33ba8bd2c942: [Windows SEH]: Fix -O2 crash for Windows -EHa 
(authored by tentzen).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103664

Files:
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7151,8 +7151,6 @@
 if (types::isCXX(InputType))
   CmdArgs.push_back("-fcxx-exceptions");
 CmdArgs.push_back("-fexceptions");
-if (EH.Asynch)
-  CmdArgs.push_back("-fasync-exceptions");
   }
   if (types::isCXX(InputType) && EH.Synch && EH.NoUnwindC)
 CmdArgs.push_back("-fexternc-nounwind");
Index: clang/lib/CodeGen/CGCleanup.cpp
===
--- clang/lib/CodeGen/CGCleanup.cpp
+++ clang/lib/CodeGen/CGCleanup.cpp
@@ -195,7 +195,7 @@
 Scope->setLifetimeMarker();
 
   // With Windows -EHa, Invoke llvm.seh.scope.begin() for EHCleanup
-  if (CGF->getLangOpts().EHAsynch && IsEHCleanup &&
+  if (CGF->getLangOpts().EHAsynch && IsEHCleanup && !IsLifetimeMarker &&
   CGF->getTarget().getCXXABI().isMicrosoft())
 CGF->EmitSehCppScopeBegin();
 


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7151,8 +7151,6 @@
 if (types::isCXX(InputType))
   CmdArgs.push_back("-fcxx-exceptions");
 CmdArgs.push_back("-fexceptions");
-if (EH.Asynch)
-  CmdArgs.push_back("-fasync-exceptions");
   }
   if (types::isCXX(InputType) && EH.Synch && EH.NoUnwindC)
 CmdArgs.push_back("-fexternc-nounwind");
Index: clang/lib/CodeGen/CGCleanup.cpp
===
--- clang/lib/CodeGen/CGCleanup.cpp
+++ clang/lib/CodeGen/CGCleanup.cpp
@@ -195,7 +195,7 @@
 Scope->setLifetimeMarker();
 
   // With Windows -EHa, Invoke llvm.seh.scope.begin() for EHCleanup
-  if (CGF->getLangOpts().EHAsynch && IsEHCleanup &&
+  if (CGF->getLangOpts().EHAsynch && IsEHCleanup && !IsLifetimeMarker &&
   CGF->getTarget().getCXXABI().isMicrosoft())
 CGF->EmitSehCppScopeBegin();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D103664#2799944 , @tentzen wrote:

> since I cannot repro it locally, let's have this patch in to resolve -EHa -O2 
> crashes for now.
> I will add more -O2 tests in following patches.

Sounds good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103664

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

[PATCH] D102736: Fix tmp files being left on Windows builds.

[Amy Huang via Phabricator via cfe-commits]

akhuang closed this revision.
akhuang added a comment.

Committed in 9d070b2f4889887f9ce497592ef01df7b9601a1c 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102736

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

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:935
+  // data variables can be private. This optimization applies on COFF and ELF.
+  if (!DataReferencedByCode && !TT.isOSBinFormatMachO()) {
+Linkage = GlobalValue::PrivateLinkage;

I am conservative here. For ELF `__profd_*` can be unconditionally private, 
even in `DataReferencedByCode == true` mode. This will decrease object file 
sizes for regular PGO.

But there may be too many changes now. I'll postpone doing this after this 
proves to be stable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103372

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

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

[Ten Tzen via Phabricator via cfe-commits]

tentzen added a comment.

since I cannot repro it locally, let's have this patch in to resolve -EHa -O2 
crashes for now.
I will add more -O2 tests in following patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103664

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

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

[Fangrui Song 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 rG9e51d1f348b9: [InstrProfiling] If no value profiling, make 
data variable private and (for… (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103372

Files:
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/comdat.ll
  llvm/test/Instrumentation/InstrProfiling/icall.ll
  llvm/test/Instrumentation/InstrProfiling/linkage.ll
  llvm/test/Instrumentation/InstrProfiling/platform.ll
  llvm/test/Instrumentation/InstrProfiling/profiling.ll
  llvm/test/Transforms/PGOProfile/comdat_internal.ll

Index: llvm/test/Transforms/PGOProfile/comdat_internal.ll
===
--- llvm/test/Transforms/PGOProfile/comdat_internal.ll
+++ llvm/test/Transforms/PGOProfile/comdat_internal.ll
@@ -7,16 +7,16 @@
 ; CHECK: $foo = comdat any
 
 ; CHECK: $__llvm_profile_raw_version = comdat any
-; CHECK: $__profd__stdin__foo.[[FOO_HASH:[0-9]+]] = comdat any
+; CHECK: $__profc__stdin__foo.[[#FOO_HASH:]] = comdat any
 
 @bar = global i32 ()* @foo, align 8
 
 ; CHECK: @__llvm_profile_raw_version = constant i64 {{[0-9]+}}, comdat
 ; CHECK-NOT: __profn__stdin__foo
-; CHECK: @__profc__stdin__foo.[[FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd__stdin__foo.[[FOO_HASH]]), align 8
-; CHECK: @__profd__stdin__foo.[[FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[FOO_HASH]], i32 0, i32 0), i8* null
+; CHECK: @__profc__stdin__foo.[[#FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; CHECK: @__profd__stdin__foo.[[#FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[#FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[#FOO_HASH]], i32 0, i32 0), i8* null
 ; CHECK-NOT: bitcast (i32 ()* @foo to i8*)
-; CHECK-SAME: , i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat, align 8
+; CHECK-SAME: , i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profc__stdin__foo.[[#FOO_HASH]]), align 8
 ; CHECK: @__llvm_prf_nm
 ; CHECK: @llvm.compiler.used
 
Index: llvm/test/Instrumentation/InstrProfiling/profiling.ll
===
--- llvm/test/Instrumentation/InstrProfiling/profiling.ll
+++ llvm/test/Instrumentation/InstrProfiling/profiling.ll
@@ -17,8 +17,8 @@
 @__profn_baz = private constant [3 x i8] c"baz"
 ; CHECK-NOT: __profn_baz
 
-; ELF:   @__profc_foo = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_foo), align 8
-; ELF:   @__profd_foo = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_foo = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_foo = private {{.*}}, section "__llvm_prf_data", comdat($__profc_foo), align 8
 ; MACHO: @__profc_foo = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__profd_foo = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
 ; WIN:   @__profc_foo = private global [1 x i64] zeroinitializer, section ".lprfc$M", align 8
@@ -28,8 +28,8 @@
   ret void
 }
 
-; ELF:   @__profc_bar = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_bar), align 8
-; ELF:   @__profd_bar = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_bar = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_bar = private {{.*}}, section "__llvm_prf_data", comdat($__profc_bar), align 8
 ; MACHO: @__profc_bar = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__profd_bar = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
 ; WIN:   @__profc_bar = private global [1 x i64] zeroinitializer, section ".lprfc$M", align 8
@@ -39,8 +39,8 @@
   ret void
 }
 
-; ELF:   @__profc_baz = private global [3 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_baz), align 8
-; ELF:   @__profd_baz = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_baz = private global [3 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_baz = private {{.*}}, section "__llvm_prf_data", comdat($__profc_baz), align 8
 ; MACHO: @__profc_baz = private global [3 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__profd_baz = private {{.*}}, sec

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 349956.
MaskRay added a comment.

better description


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103372

Files:
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/comdat.ll
  llvm/test/Instrumentation/InstrProfiling/icall.ll
  llvm/test/Instrumentation/InstrProfiling/linkage.ll
  llvm/test/Instrumentation/InstrProfiling/platform.ll
  llvm/test/Instrumentation/InstrProfiling/profiling.ll
  llvm/test/Transforms/PGOProfile/comdat_internal.ll

Index: llvm/test/Transforms/PGOProfile/comdat_internal.ll
===
--- llvm/test/Transforms/PGOProfile/comdat_internal.ll
+++ llvm/test/Transforms/PGOProfile/comdat_internal.ll
@@ -7,16 +7,16 @@
 ; CHECK: $foo = comdat any
 
 ; CHECK: $__llvm_profile_raw_version = comdat any
-; CHECK: $__profd__stdin__foo.[[FOO_HASH:[0-9]+]] = comdat any
+; CHECK: $__profc__stdin__foo.[[#FOO_HASH:]] = comdat any
 
 @bar = global i32 ()* @foo, align 8
 
 ; CHECK: @__llvm_profile_raw_version = constant i64 {{[0-9]+}}, comdat
 ; CHECK-NOT: __profn__stdin__foo
-; CHECK: @__profc__stdin__foo.[[FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd__stdin__foo.[[FOO_HASH]]), align 8
-; CHECK: @__profd__stdin__foo.[[FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[FOO_HASH]], i32 0, i32 0), i8* null
+; CHECK: @__profc__stdin__foo.[[#FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; CHECK: @__profd__stdin__foo.[[#FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[#FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[#FOO_HASH]], i32 0, i32 0), i8* null
 ; CHECK-NOT: bitcast (i32 ()* @foo to i8*)
-; CHECK-SAME: , i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat, align 8
+; CHECK-SAME: , i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profc__stdin__foo.[[#FOO_HASH]]), align 8
 ; CHECK: @__llvm_prf_nm
 ; CHECK: @llvm.compiler.used
 
Index: llvm/test/Instrumentation/InstrProfiling/profiling.ll
===
--- llvm/test/Instrumentation/InstrProfiling/profiling.ll
+++ llvm/test/Instrumentation/InstrProfiling/profiling.ll
@@ -17,8 +17,8 @@
 @__profn_baz = private constant [3 x i8] c"baz"
 ; CHECK-NOT: __profn_baz
 
-; ELF:   @__profc_foo = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_foo), align 8
-; ELF:   @__profd_foo = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_foo = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_foo = private {{.*}}, section "__llvm_prf_data", comdat($__profc_foo), align 8
 ; MACHO: @__profc_foo = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__profd_foo = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
 ; WIN:   @__profc_foo = private global [1 x i64] zeroinitializer, section ".lprfc$M", align 8
@@ -28,8 +28,8 @@
   ret void
 }
 
-; ELF:   @__profc_bar = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_bar), align 8
-; ELF:   @__profd_bar = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_bar = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_bar = private {{.*}}, section "__llvm_prf_data", comdat($__profc_bar), align 8
 ; MACHO: @__profc_bar = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__profd_bar = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
 ; WIN:   @__profc_bar = private global [1 x i64] zeroinitializer, section ".lprfc$M", align 8
@@ -39,8 +39,8 @@
   ret void
 }
 
-; ELF:   @__profc_baz = private global [3 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_baz), align 8
-; ELF:   @__profd_baz = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_baz = private global [3 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_baz = private {{.*}}, section "__llvm_prf_data", comdat($__profc_baz), align 8
 ; MACHO: @__profc_baz = private global [3 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__profd_baz = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
 ; WIN:   @__profc_baz = private global [3 x i64] zeroinitializer, section ".lprfc$M", align 8
@@ -56,7 +56,7 @@

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:127
+/// into a special block region.
+BlockCapture
+  };

xazax.hun wrote:
> There are some other places were we might have effects that are very similar 
> to stores, like invalidations during conservative function evaluation. 
Yes, this would be really interesting.  Like, we probably don't want to add 
notes saying "we invalidated the Store so here's why we're now assuming stuff 
that seemingly contradicts our previous assumptions". But we may want to add 
//suppressions// based on this info, eg. "our control flow dependencies are 
based on invalidation, maybe let's try to find a different path to the same 
bug?"



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:210
+  ///much.
+  virtual Result track(KnownSVal V, const MemRegion *R,
+   TrackingOptions Opts = {},

xazax.hun wrote:
> Not directly related to this patch, but I wonder if we want to have unknown 
> values with identities at some point, so we can track them.
`UnknownVal` is a stop-gap for situations when we've screwed up so badly we 
don't even have types anymore. The only thing I'd ever want from them is to 
disappear :)

I guess this could be useful for a debug checker that could explain where does 
an `UnknownVal` come from. In this case unknown values don't need identities; 
we can track other values without identities, such as null pointers, just fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103605

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

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D103664#2799852 , @tentzen wrote:

> @zahiraam, are you removing all those CHECKs:
>
> - CHECK: invoke void @llvm.seh.scope.**
>
> those are placed there to ensure SEH scope semantic is preserved for Od..

I was just showing you. The checks generated at O2 
 are only the ones that I have 
left. At O0 the checks need to stay. So you need to rewrite the LIT tests so 
that both are expected for one option and the other.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103664

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

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 349953.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Exclude Mach-O.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103372

Files:
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/comdat.ll
  llvm/test/Instrumentation/InstrProfiling/icall.ll
  llvm/test/Instrumentation/InstrProfiling/linkage.ll
  llvm/test/Instrumentation/InstrProfiling/platform.ll
  llvm/test/Instrumentation/InstrProfiling/profiling.ll
  llvm/test/Transforms/PGOProfile/comdat_internal.ll

Index: llvm/test/Transforms/PGOProfile/comdat_internal.ll
===
--- llvm/test/Transforms/PGOProfile/comdat_internal.ll
+++ llvm/test/Transforms/PGOProfile/comdat_internal.ll
@@ -7,16 +7,16 @@
 ; CHECK: $foo = comdat any
 
 ; CHECK: $__llvm_profile_raw_version = comdat any
-; CHECK: $__profd__stdin__foo.[[FOO_HASH:[0-9]+]] = comdat any
+; CHECK: $__profc__stdin__foo.[[#FOO_HASH:]] = comdat any
 
 @bar = global i32 ()* @foo, align 8
 
 ; CHECK: @__llvm_profile_raw_version = constant i64 {{[0-9]+}}, comdat
 ; CHECK-NOT: __profn__stdin__foo
-; CHECK: @__profc__stdin__foo.[[FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd__stdin__foo.[[FOO_HASH]]), align 8
-; CHECK: @__profd__stdin__foo.[[FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[FOO_HASH]], i32 0, i32 0), i8* null
+; CHECK: @__profc__stdin__foo.[[#FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; CHECK: @__profd__stdin__foo.[[#FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[#FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[#FOO_HASH]], i32 0, i32 0), i8* null
 ; CHECK-NOT: bitcast (i32 ()* @foo to i8*)
-; CHECK-SAME: , i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat, align 8
+; CHECK-SAME: , i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profc__stdin__foo.[[#FOO_HASH]]), align 8
 ; CHECK: @__llvm_prf_nm
 ; CHECK: @llvm.compiler.used
 
Index: llvm/test/Instrumentation/InstrProfiling/profiling.ll
===
--- llvm/test/Instrumentation/InstrProfiling/profiling.ll
+++ llvm/test/Instrumentation/InstrProfiling/profiling.ll
@@ -17,8 +17,8 @@
 @__profn_baz = private constant [3 x i8] c"baz"
 ; CHECK-NOT: __profn_baz
 
-; ELF:   @__profc_foo = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_foo), align 8
-; ELF:   @__profd_foo = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_foo = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_foo = private {{.*}}, section "__llvm_prf_data", comdat($__profc_foo), align 8
 ; MACHO: @__profc_foo = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__profd_foo = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
 ; WIN:   @__profc_foo = private global [1 x i64] zeroinitializer, section ".lprfc$M", align 8
@@ -28,8 +28,8 @@
   ret void
 }
 
-; ELF:   @__profc_bar = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_bar), align 8
-; ELF:   @__profd_bar = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_bar = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_bar = private {{.*}}, section "__llvm_prf_data", comdat($__profc_bar), align 8
 ; MACHO: @__profc_bar = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__profd_bar = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
 ; WIN:   @__profc_bar = private global [1 x i64] zeroinitializer, section ".lprfc$M", align 8
@@ -39,8 +39,8 @@
   ret void
 }
 
-; ELF:   @__profc_baz = private global [3 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_baz), align 8
-; ELF:   @__profd_baz = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_baz = private global [3 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_baz = private {{.*}}, section "__llvm_prf_data", comdat($__profc_baz), align 8
 ; MACHO: @__profc_baz = private global [3 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__profd_baz = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
 ; WIN:   @__profc_baz = private global [3 x i64] zeroinitializer, sec

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D103605#2798581 , @vsavchenko 
wrote:

> In D103605#2798171 , @NoQ wrote:
>
>> - Trackers are attached to bug reports. A single bug report may have 
>> multiple trackers.
>>   - Why not have only one tracker per report?
>
> That's a good question, we can actually even make it a part of `BugReport` 
> for that matter.  I can though think of one exotic case, where you wait till 
> you encounter certain weird condition, where you need to spawn a very 
> specific customized tracker only starting from that point.

Ok, let's take a simpler example:

  int foo(int D) {
int A = 1;
int B = 1;
int C = B - A;
return D / C; // division by zero
  }

And suppose we want to track `A` and `B` so that to explain why we think that 
`C` is zero. What are pros and cons of:
(1) tracking `A` and `B` within the same tracker that already helped us reach 
`C`?
(2) terminating the current tracker as it reaches `C` and spawning two separate 
trackers for `A` and `B`?

>> - The termination condition is as usual: no further visitors attached.
>>   - It sounds like this patchset is orthogonal to the problem of detecting 
>> the tracking termination point(?) It does make it easier to implement 
>> customized reactions to such termination though.
>
> Actually, tracker dies when this happens.  But hey, can you provide a use 
> case when the checker really needs to know when this happens?

Inlined defensive check suppressions are a major example. Basically null 
dereference warnings are suppressed based on properties of the program point at 
which the tracker terminates. If we know when this happens, we can refactor 
inlined defensive check suppressions into a checker specific handler.

>> - However, instead of adding notes directly, they simply invoke `track()`. 
>> All note-attaching work is moved into additional default handlers.
>
> No, they call `track` where they used to call `trackExpressionValue` or add 
> `FindLastStoreBRVisitor`.
> Right now, only stores received their special customization and can produce 
> notes based on tracker customization.

What's the ultimate plan though, do we want to fully move all notes into 
handlers?

>> - Let's consider an example. Suppose you're implementing `evalCall` for 
>> `llvm::dyn_cast`. You could have your expression handler detect a modeled 
>> `dyn_cast` call and implement its `handle()` method to emit a 
>> checker-specific note and start tracking the argument.
>
> So, in here it would look like this: If I want to support `dyn_cast` for 
> tracking, I will add extra `ExpressionHandler` that calls `track` on the 
> expression inside of `llvm::dyn_cast`.  Additionally, if you want to change 
> the note's message for situations like `a = dyn_cast(b)`, you can add a 
> `StoreHandler` that will produce a note like "a is casted from b".
>
>> - With this implementation such note will only be emitted if the value is 
>> tracked back to `dyn_cast`. Not every modeled `dyn_cast` will receive a note 
>> but only the one that, say, caused a null dereference on its return value 
>> when it failed.
>
> Correct.
>
>> - Now, how does this interact with note tags? The idea behind note tags is 
>> so that visitors didn't need to reverse-engineer the analysis.
>
> I mean, it's just an orthogonal mechanism.
>
>> - It sounds like the handler is a step back to the visitor world: it doesn't 
>> know how exactly was `dyn_cast` modeled during analysis, so it has to 
>> reverse-engineer what happened.
>
> Correct, but during the analysis, checker could've modeled a zillion of 
> different `dyn_casts`, and we don't want to have notes on all of them.
>
>> - For example, in order to find out whether `dyn_cast` succeeded or failed, 
>> it has to explore the program state to find out which values are passed into 
>> the cast and returned from the cast.
>> - Whereas a note tag would have this information accessible directly.
>
> Maybe we can access the note tag from the handler and kill two birds with one 
> stone?

If we can, one way or another, I believe we absolutely should kill two birds 
with one stone. We shouldn't have two mutually exclusive solutions for two 
aspects of the same problem. In order to produce good notes, it's necessary to 
*both* know what happened during analysis *and* know whether the value is 
tracked.

>> - Hence a suggestion. Maybe instead of having a system of handlers, allow 
>> visitors to //ask// the tracker whether `track()` was invoked at this node 
>> on a certain expression or region?
>
> There are still situations when you don't have tags, but want to hook into 
> the tracking process, so you can add your custom feature.  Let's say you 
> modeled a system call `foo(a, b)` and you want to track `a` and `b`.  Or more 
> generally if you want to help tracking when it's stuck.

Yes, so like in my `a = dyn_cast<...>(b)` example the proposed solution would 
be like:

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

[Ten Tzen via Phabricator via cfe-commits]

tentzen added a comment.

@zahiraam, are you removing all those CHECKs:

- CHECK: invoke void @llvm.seh.scope.**

those are placed there to ensure SEH scope semantic is preserved for Od..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103664

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

[PATCH] D103184: [AArch64] handle -Wa,-march=

[Jian Cai via Phabricator via cfe-commits]

jcai19 added a comment.

In D103184#2799732 , @nickdesaulniers 
wrote:

> That looks better, thanks @jcai19 .  Based on @DavidSpickett 's LGTM I think 
> this is now ready to land.
>
> @DavidSpickett did ask:
>
>> Also please include in the commit message the "rules" that we're using here 
>> for parsing. Like I did for the arm change.
>
> So you may want to edit the commit description manually in phab if you plan 
> to `arc patch` this in before merging, otherwise please do so locally before 
> merging to main then pushing.

Thanks for the confirmation! I've edited the commit message to include the 
rules per discussed. Will wait till Monday in case @DavidSpickett has more 
comments.




Comment at: clang/test/Driver/aarch64-target-as-march.s:29
+// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
+
+// MULTIPLE-VALUES: "-target-feature" "+v8.1a

nickdesaulniers wrote:
> jcai19 wrote:
> > DavidSpickett wrote:
> > > Add a test with `-Wa,-march=armv8.2-a,-march=armv8.1-a` (one -Wa with 
> > > multiple values attached), then repeat the two tests but with 
> > > `-Xassembler` instead.
> > I added a test case for -Xassembler -march=armv8.2-a,-march=armv8.1-a,  but 
> > it seems it does not support multiple values in one invocation?
> > 
> > clang --target=aarch64-linux-gnueabi -Xassembler 
> > -march=armv8.2-a,-march=armv8.1-a -c foo.s -o ias.o -###
> > ...
> > clang-13: error: the clang compiler does not support '-Xassembler 
> > -march=armv8.2-a,-march=armv8.1-a'
> > ...
> I don't think @DavidSpickett meant more `UNUSED-WARNING` tests, but tests 
> that check the actual chosen value.
That makes more sense. Sorry for the noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

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

[PATCH] D103485: Fix a diagnoses-valid bug with using declarations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thanks for the review! I've commit in ca68f3bc48e48f839142de1461e95d87ae48e9df 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103485

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

[clang] ca68f3b - Fix a diagnoses-valid bug with using declarations

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2021-06-04T15:52:07-04:00
New Revision: ca68f3bc48e48f839142de1461e95d87ae48e9df

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

LOG: Fix a diagnoses-valid bug with using declarations

The following was found by a customer and is accepted by the other primary
C++ compilers, but fails to compile in Clang:

namespace sss {
double foo(int, double);
template 
T foo(T); // note: target of using declaration
}  // namespace sss

namespace oad {
void foo();
}

namespace oad {
using ::sss::foo;
}

namespace sss {
using oad::foo; // note: using declaration
}

namespace sss {
double foo(int, double) { return 0; }
template 
T foo(T t) { // error: declaration conflicts with target of using
  return t;
}
}  // namespace sss

I believe the issue is that MergeFunctionDecl() was calling
checkUsingShadowRedecl() but only considering a FunctionDecl as a
possible shadow and not FunctionTemplateDecl. The changes in this patch
largely mirror how variable declarations were being handled by also
catching FunctionTemplateDecl.

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp
clang/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp
clang/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-global.cpp
clang/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
clang/test/SemaCXX/using-decl-templates.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0abdd31b43ec3..4f62113dd8e60 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3165,6 +3165,7 @@ static bool haveIncompatibleLanguageLinkages(const T 
*Old, const T *New) {
 
 template static bool isExternC(T *D) { return D->isExternC(); }
 static bool isExternC(VarTemplateDecl *) { return false; }
+static bool isExternC(FunctionTemplateDecl *) { return false; }
 
 /// Check whether a redeclaration of an entity introduced by a
 /// using-declaration is valid, given that we know it's not an overload
@@ -3289,10 +3290,20 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, 
NamedDecl *&OldD,
 return true;
   }
 
-  // Check whether the two declarations might declare the same function.
-  if (checkUsingShadowRedecl(*this, Shadow, New))
-return true;
-  OldD = Old = cast(Shadow->getTargetDecl());
+  // Check whether the two declarations might declare the same function or
+  // function template.
+  if (FunctionTemplateDecl *NewTemplate =
+  New->getDescribedFunctionTemplate()) {
+if (checkUsingShadowRedecl(*this, Shadow,
+ NewTemplate))
+  return true;
+OldD = Old = cast(Shadow->getTargetDecl())
+ ->getAsFunction();
+  } else {
+if (checkUsingShadowRedecl(*this, Shadow, New))
+  return true;
+OldD = Old = cast(Shadow->getTargetDecl());
+  }
 } else {
   Diag(New->getLocation(), diag::err_redefinition_
diff erent_kind)
 << New->getDeclName();

diff  --git 
a/clang/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp 
b/clang/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp
index cceca5106cae9..a16374a80099e 100644
--- a/clang/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp
+++ b/clang/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp
@@ -9,7 +9,7 @@ struct str; // expected-note {{previous declaration is here}}
 using type = int;
 
 template extern int var_tpl; // expected-note {{previous declaration 
is here}}
-template int func_tpl(); // expected-note-re previous 
declaration is here|target of using declaration
+template int func_tpl(); // expected-note {{previous declaration is 
here}}
 template struct str_tpl; // expected-note {{previous declaration is 
here}}
 template using type_tpl = int; // expected-note {{previous 
declaration is here}}
 
@@ -26,7 +26,7 @@ using ::func;
 using ::str;
 using ::type;
 using ::var_tpl;
-using ::func_tpl; // expected-note {{using declaration}}
+using ::func_tpl;
 using ::str_tpl;
 using ::type_tpl;
 #endif
@@ -41,8 +41,7 @@ struct str; // expected-error {{declaration of 'str' in 
module M follows declara
 using type = int;
 
 template extern int var_tpl; // expected-error {{declaration of 
'var_tpl' in module M follows declaration in the global module}}
-// FIXME: Is this the right diagnostic in the -DUSING case?
-template int func_tpl(); // expected-error-re declaration of 
'func_tpl' in module M follows declaration in the global module|conflicts with 
target of using declaration
+template int func_tpl(); // expected-error {{declaration of 
'func_tpl' in module M follows declaration in the global module}}
 temp

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:110
+  /// (inlined defensive checks, returned null).
+  bool EnableNullFPSuppression = true;
+};

I vaguely remember we wanting to put this defensive check suppression in the 
related checkers. If that is the case, I wonder if this option belongs here. 



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:127
+/// into a special block region.
+BlockCapture
+  };

There are some other places were we might have effects that are very similar to 
stores, like invalidations during conservative function evaluation. 



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:147
+class ExpressionHandler;
+class StoreHandler;
+

During path sensitive analysis we already have a callback for stores. We kind 
of replicating this logic for bug paths. 
So my questions are:
* Do we expect to have additional information here that were not available 
during the analysis earlier?
* Do we want to make this as similar to the forward analysis part as possible 
for developer familiarity?



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:210
+  ///much.
+  virtual Result track(KnownSVal V, const MemRegion *R,
+   TrackingOptions Opts = {},

Not directly related to this patch, but I wonder if we want to have unknown 
values with identities at some point, so we can track them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103605

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

[PATCH] D103611: Correct the behavior of va_arg checking in C++

[Aaron Ballman via Phabricator via cfe-commits]

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

Changed the test case to speculatively see if the CI pipeline is happy with 
this (I'm trying to avoid adding a triple to the test).


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

https://reviews.llvm.org/D103611

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/varargs.cpp


Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++03 -verify %s
+// RUN: %clang_cc1 -std=c++03 -Wno-c++11-extensions -verify %s
 // RUN: %clang_cc1 -std=c++11 -verify %s
 
 __builtin_va_list ap;
@@ -28,6 +28,30 @@
   };
 }
 
+// Ensure the correct behavior for promotable type UB checking.
+void promotable(int a, ...) {
+  enum Unscoped { One = 0x7FFF };
+  (void)__builtin_va_arg(ap, Unscoped); // ok
+
+  enum class Scoped { Two };
+  (void)__builtin_va_arg(ap, Scoped); // ok
+
+  enum Fixed : int { Three };
+  (void)__builtin_va_arg(ap, Fixed); // ok
+
+  enum FixedSmall : char { Four };
+  (void)__builtin_va_arg(ap, FixedSmall); // expected-warning {{second 
argument to 'va_arg' is of promotable type 'FixedSmall'; this va_arg has 
undefined behavior because arguments will be promoted to 'int'}}
+
+  enum FixedLarge : long long { Five };
+  (void)__builtin_va_arg(ap, FixedLarge); // ok
+
+  // Ensure that qualifiers are ignored.
+  (void)__builtin_va_arg(ap, const volatile int);  // ok
+
+  // Ensure that signed vs unsigned doesn't matter either.
+  (void)__builtin_va_arg(ap, unsigned int);
+}
+
 #if __cplusplus >= 201103L
 // We used to have bugs identifying the correct enclosing function scope in a
 // lambda.
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15750,7 +15750,29 @@
 QualType PromoteType;
 if (TInfo->getType()->isPromotableIntegerType()) {
   PromoteType = Context.getPromotedIntegerType(TInfo->getType());
-  if (Context.typesAreCompatible(PromoteType, TInfo->getType()))
+  // [cstdarg.syn]p1 defers the C++ behavior to what the C standard says,
+  // and C2x 7.16.1.1p2 says, in part:
+  //   If type is not compatible with the type of the actual next argument
+  //   (as promoted according to the default argument promotions), the
+  //   behavior is undefined, except for the following cases:
+  // - both types are pointers to qualified or unqualified versions of
+  //   compatible types;
+  // - one type is a signed integer type, the other type is the
+  //   corresponding unsigned integer type, and the value is
+  //   representable in both types;
+  // - one type is pointer to qualified or unqualified void and the
+  //   other is a pointer to a qualified or unqualified character type.
+  // Given that type compatibility is the primary requirement (ignoring
+  // qualifications), you would think we could call typesAreCompatible()
+  // directly to test this. However, in C++, that checks for *same type*,
+  // which causes false positives when passing an enumeration type to
+  // va_arg. Instead, get the underlying type of the enumeration and pass
+  // that.
+  QualType UnderlyingType = TInfo->getType();
+  if (const auto *ET = UnderlyingType->getAs())
+UnderlyingType = ET->getDecl()->getIntegerType();
+  if (Context.typesAreCompatible(PromoteType, UnderlyingType,
+ /*CompareUnqualified*/ true))
 PromoteType = QualType();
 }
 if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float))


Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++03 -verify %s
+// RUN: %clang_cc1 -std=c++03 -Wno-c++11-extensions -verify %s
 // RUN: %clang_cc1 -std=c++11 -verify %s
 
 __builtin_va_list ap;
@@ -28,6 +28,30 @@
   };
 }
 
+// Ensure the correct behavior for promotable type UB checking.
+void promotable(int a, ...) {
+  enum Unscoped { One = 0x7FFF };
+  (void)__builtin_va_arg(ap, Unscoped); // ok
+
+  enum class Scoped { Two };
+  (void)__builtin_va_arg(ap, Scoped); // ok
+
+  enum Fixed : int { Three };
+  (void)__builtin_va_arg(ap, Fixed); // ok
+
+  enum FixedSmall : char { Four };
+  (void)__builtin_va_arg(ap, FixedSmall); // expected-warning {{second argument to 'va_arg' is of promotable type 'FixedSmall'; this va_arg has undefined behavior because arguments will be promoted to 'int'}}
+
+  enum FixedLarge : long long { Five };
+  (void)__builtin_va_arg(ap, FixedLarge); // ok
+
+  // Ensure that qualifiers are ignored.
+  (void)__builtin_va_arg(ap, const volatile int

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D103664#2799725 , @tentzen wrote:

> In D103664#2798732 , @aganea wrote:
>
>> Thanks for the quick fix! Would you mind fixing the two failing tests 
>> please? (see above)
>
> Hmm, I cannot repro locally..
>
> @zahiraam, currently -EHa is not completely ready yet. we need a couple of 
> more patches.  So turn it off to preserve zero-diff, unless that 
> -fasync-exceptions is explicitly specified.

Got it. Thanks. 
I was able to repro the fails. And this is the fix for it:
diff --git a/clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp 
b/clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp
index 47117b4a9613..b9195a2920e6 100644

- a/clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp

+++ b/clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp
@@ -1,60 +1,54 @@
 // RUN: %clang_cc1 -triple x86_64-windows -fasync-exceptions -fcxx-exceptions 
-fexceptions -fms-extensions -x c++ -Wno-implicit-function-declaration -S 
-emit-llvm %s -o - | FileCheck %s

-

-// CHECK: invoke void @llvm.seh.scope.begin()
-// CHECK: invoke void @llvm.seh.scope.begin()
-// CHECK: invoke void @llvm.seh.scope.begin()
-// CHECK: invoke void @llvm.seh.scope.end()
-// CHECK: invoke void @llvm.seh.scope.end()
-// CHECK: invoke void @llvm.seh.scope.end()
+// RUN: %clang_cc1 -triple x86_64-windows -fasync-exceptions -fcxx-exceptions 
-fexceptions -fms-extensions -x c++ -Wno-implicit-function-declaration -O2 -S 
-emit-llvm %s -o - | FileCheck %s

// CHECK: invoke void @llvm.seh.try.begin()
-// CHECK: %[[src:[0-9-]+]] = load volatile i32, i32* %i
-// CHECK-NEXT: invoke void @"?crash@@YAXH@Z"(i32 %[[src]])
+// CHECK: = load volatile i32, i32* %i
+// CHECK-NEXT: invoke void @"?crash@@YAXH@Z"(i32
 // CHECK: invoke void @llvm.seh.try.end()

I was able to repro the fail and this is the fix for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103664

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

[PATCH] D103663: [AMDGPU] Add gfx1013 target

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.intersect_ray.ll:4
+; RUN: llc -global-isel -march=amdgcn -mcpu=gfx1013 -verify-machineinstrs < %s 
| FileCheck -check-prefix=GCN %s
+; RUN: llc -global-isel -march=amdgcn -mcpu=gfx1012 -verify-machineinstrs < %s 
| FileCheck -check-prefix=GCN %s
 

rampitec wrote:
> foad wrote:
> > This test surely should not pass for gfx1012, since it does not have these 
> > instructions. And with your patch as written it should fail for gfx1013 
> > too, since they are predicated on HasGFX10_BEncoding.
> > 
> > @rampitec any idea what is wrong here? Apparently the backend will happily 
> > generate image_bvh_intersect_ray instructions even for gfx900!
> Indeed. MIMG_IntersectRay has this:
> 
> ```
>   let SubtargetPredicate = HasGFX10_BEncoding,
>   AssemblerPredicate = HasGFX10_BEncoding,
> ```
> but apparently SubtargetPredicate did not work. It needs to be fixed.
> 
> gfx1012 does not have it, gfx1013 does though. That is what GFX10_A encoding 
> is about, 10_B it has to be replaced with 10_A in BVH and MSAA load.
Image lowering and selection is not really done like everything else. For BVH 
it just lowers intrinsic to opcode. I think the easiest fix is to add to 
SIISelLowering.cpp where we lower Intrinsic::amdgcn_image_bvh_intersect_ray 
something like this:


```
  if (!Subtarget->hasGFX10_AEncoding())
report_fatal_error(
"requested image instruction is not supported on this GPU");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103663

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

[PATCH] D103184: [AArch64] handle -Wa,-march=

[Nick Desaulniers via Phabricator via cfe-commits]

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

That looks better, thanks @jcai19 .  Based on @DavidSpickett 's LGTM I think 
this is now ready to land.

@DavidSpickett did ask:

> Also please include in the commit message the "rules" that we're using here 
> for parsing. Like I did for the arm change.

So you may want to edit the commit description manually in phab if you plan to 
`arc patch` this in before merging, otherwise please do so locally before 
merging to main then pushing.




Comment at: clang/test/Driver/aarch64-target-as-march.s:29
+// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
+
+// MULTIPLE-VALUES: "-target-feature" "+v8.1a

nickdesaulniers wrote:
> jcai19 wrote:
> > DavidSpickett wrote:
> > > Add a test with `-Wa,-march=armv8.2-a,-march=armv8.1-a` (one -Wa with 
> > > multiple values attached), then repeat the two tests but with 
> > > `-Xassembler` instead.
> > I added a test case for -Xassembler -march=armv8.2-a,-march=armv8.1-a,  but 
> > it seems it does not support multiple values in one invocation?
> > 
> > clang --target=aarch64-linux-gnueabi -Xassembler 
> > -march=armv8.2-a,-march=armv8.1-a -c foo.s -o ias.o -###
> > ...
> > clang-13: error: the clang compiler does not support '-Xassembler 
> > -march=armv8.2-a,-march=armv8.1-a'
> > ...
> I don't think @DavidSpickett meant more `UNUSED-WARNING` tests, but tests 
> that check the actual chosen value.
> I added a test case for -Xassembler -march=armv8.2-a,-march=armv8.1-a, but it 
> seems it does not support multiple values in one invocation?

Looking at https://reviews.llvm.org/D95872, your assessment seems correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

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

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

[Ten Tzen via Phabricator via cfe-commits]

tentzen updated this revision to Diff 349939.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103664

Files:
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7151,8 +7151,6 @@
 if (types::isCXX(InputType))
   CmdArgs.push_back("-fcxx-exceptions");
 CmdArgs.push_back("-fexceptions");
-if (EH.Asynch)
-  CmdArgs.push_back("-fasync-exceptions");
   }
   if (types::isCXX(InputType) && EH.Synch && EH.NoUnwindC)
 CmdArgs.push_back("-fexternc-nounwind");
Index: clang/lib/CodeGen/CGCleanup.cpp
===
--- clang/lib/CodeGen/CGCleanup.cpp
+++ clang/lib/CodeGen/CGCleanup.cpp
@@ -195,7 +195,7 @@
 Scope->setLifetimeMarker();
 
   // With Windows -EHa, Invoke llvm.seh.scope.begin() for EHCleanup
-  if (CGF->getLangOpts().EHAsynch && IsEHCleanup &&
+  if (CGF->getLangOpts().EHAsynch && IsEHCleanup && !IsLifetimeMarker &&
   CGF->getTarget().getCXXABI().isMicrosoft())
 CGF->EmitSehCppScopeBegin();
 


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7151,8 +7151,6 @@
 if (types::isCXX(InputType))
   CmdArgs.push_back("-fcxx-exceptions");
 CmdArgs.push_back("-fexceptions");
-if (EH.Asynch)
-  CmdArgs.push_back("-fasync-exceptions");
   }
   if (types::isCXX(InputType) && EH.Synch && EH.NoUnwindC)
 CmdArgs.push_back("-fexternc-nounwind");
Index: clang/lib/CodeGen/CGCleanup.cpp
===
--- clang/lib/CodeGen/CGCleanup.cpp
+++ clang/lib/CodeGen/CGCleanup.cpp
@@ -195,7 +195,7 @@
 Scope->setLifetimeMarker();
 
   // With Windows -EHa, Invoke llvm.seh.scope.begin() for EHCleanup
-  if (CGF->getLangOpts().EHAsynch && IsEHCleanup &&
+  if (CGF->getLangOpts().EHAsynch && IsEHCleanup && !IsLifetimeMarker &&
   CGF->getTarget().getCXXABI().isMicrosoft())
 CGF->EmitSehCppScopeBegin();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

[Ten Tzen via Phabricator via cfe-commits]

tentzen added a comment.

In D103664#2798732 , @aganea wrote:

> Thanks for the quick fix! Would you mind fixing the two failing tests please? 
> (see above)

Hmm, I cannot repro locally..

@zahiraam, currently -EHa is not completely ready yet. we need a couple of more 
patches.  So turn it off to preserve zero-diff, unless that -fasync-exceptions 
is explicitly specified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103664

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

[PATCH] D103184: [AArch64] handle -Wa,-march=

[Jian Cai via Phabricator via cfe-commits]

jcai19 updated this revision to Diff 349938.
jcai19 marked an inline comment as done.
jcai19 added a comment.

Update test cases based on comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/aarch64-target-as-march.s

Index: clang/test/Driver/aarch64-target-as-march.s
===
--- /dev/null
+++ clang/test/Driver/aarch64-target-as-march.s
@@ -0,0 +1,46 @@
+/// These tests make sure that options passed to the assembler
+/// via -Wa or -Xassembler are applied correctly to assembler inputs.
+
+/// Does not apply to non assembly files
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.1-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-1 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.1-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-1 %s
+
+// TARGET-FEATURE-1-NOT: "-target-feature" "+v8.1a"
+
+/// Does apply to assembler input
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-2 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-2 %s
+
+// TARGET-FEATURE-2: "-target-feature" "+v8.2a"
+
+/// No unused argument warnings when there are multiple values
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.1-a -Wa,-march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=UNUSED-WARNING %s
+
+// UNUSED-WARNING-NOT: warning: argument unused during compilation
+
+/// Last march to assembler wins
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.2-a -Wa,-march=armv8.1-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.2-a,-march=armv8.1-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.2-a -Xassembler \
+// RUN: -march=armv8.1-a %s 2>&1 | FileCheck --check-prefix=MULTIPLE-VALUES %s
+
+// MULTIPLE-VALUES: "-target-feature" "+v8.1a
+// MULTIPLE-VALUES-NOT: "-target-feature" "+v8.2a
+
+/// march to compiler and assembler, we choose the one suited to the input file type
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-3 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-4 %s
+
+// TARGET-FEATURE-3: "-target-feature" "+v8.3a"
+// TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
+// TARGET-FEATURE-4: "-target-feature" "+v8.4a"
+// TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -344,7 +344,7 @@
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_32:
   case llvm::Triple::aarch64_be:
-aarch64::getAArch64TargetFeatures(D, Triple, Args, Features);
+aarch64::getAArch64TargetFeatures(D, Triple, Args, Features, ForAS);
 break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
Index: clang/lib/Driver/ToolChains/Arch/AArch64.h
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.h
+++ clang/lib/Driver/ToolChains/Arch/AArch64.h
@@ -22,7 +22,8 @@
 
 void getAArch64TargetFeatures(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args,
-  std::vector &Features);
+  std::vector &Features,
+  bool ForAS);
 
 std::string getAArch64TargetCPU(const llvm::opt::ArgList &Args,
 const llvm::Triple &Triple, llvm::opt::Arg *&A);
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -185,12 +185,20 @@
 void aarch64::getAArch64TargetFeatures(const Driver &D,
const llvm::Triple &Triple,
const ArgList &Args,
-   std::vector &Features) {
+   std::vector &Features,
+   bool ForAS) {
   Arg *A;
 

[PATCH] D103082: [AArch64][SVE] Improve codegen for dupq SVE ACLE intrinsics

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma 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/D103082/new/

https://reviews.llvm.org/D103082

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

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I was wondering, if we could try something new with the tests. To increase our 
confidence that the expected behavior is correct, how about including a Z3 
proof with each of the test cases?

For example:

  (declare-const a ( _ BitVec 32 ))
  (declare-const b ( _ BitVec 32 ))
  
  ; Assume 0 <= a, b, <= 5
  (assert (bvsge a #x))
  (assert (bvsge b #x))
  (assert (bvsle a #x0005))
  (assert (bvsle b #x0005))
  
  ; Check if 0 <= a + b  <= 10
  (assert (not (and (bvsle (bvadd a b) #x0010) (bvsge (bvadd a b) 
#x
  
  (check-sat)

Of course, the example above is obvious, but with overflows and other corner 
cases it is great to have a tool doublecheck our assumptions, and share how to 
reproduce the results.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103440

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

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I think MachO doesn't have comdats, so we need to leave the `__profd_` linkage 
as it was. I think private linkage also prevents atomization, which inhibits GC 
/ dead stripping. I don't think this change really makes sense for MachO, so 
you could reasonably reland with that change.




Comment at: llvm/test/Instrumentation/InstrProfiling/linkage.ll:64
 ; MACHO: @__profc_foo_inline = linkonce_odr hidden global
-; MACHO: @__profd_foo_inline = linkonce_odr hidden global
+; MACHO: @__profd_foo_inline = private global
 ; COFF: @__profc_foo_inline = linkonce_odr hidden global{{.*}} section 
".lprfc$M", align 8

I think we don't want this change for macho. If we make `__profd_` private on 
Mac, the linker won't be able to deduplicate these `__profd` globals, and there 
will be multiple copies pointing to the same counters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103372

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

[PATCH] D103707: Add STDC macros

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan created this revision.
Herald added subscribers: jfb, kbarton, nemanjai.
Jake-Egan 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/D103707

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/test/Preprocessor/init-ppc.c


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -723,6 +723,25 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
PPC-AIX-NOTHREADSAFE %s
 // PPC-AIX-NOTHREADSAFE-NOT:#define _THREAD_SAFE 1
 
+// RUN: %clang_cc1 -x c -std=c11 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=c1x -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=iso9899:2011 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.10.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=gnu11 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=c17 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=iso9899:2017 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=c18 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=iso9899:2018 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=gnu17 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=gnu18 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=c2x -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=gnu2x -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// PPC-AIX-STDC:#define __STDC_NO_ATOMICS__ 1
+// PPC-AIX-STDC:#define __STDC_NO_THREADS__ 1
+
+// RUN: %clang_cc1 -x c -std=c99 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC-N %s
+// PPC-AIX-STDC-N-NOT:#define __STDC_NO_ATOMICS__ 1
+// PPC-AIX-STDC-N-NOT:#define __STDC_NO_THREADS__ 1
+
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-unknown-linux-gnu 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
PPC-LINUX %s
 //
 // PPC-LINUX:#define _ARCH_PPC 1
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -676,6 +676,11 @@
 
 Builder.defineMacro("_AIX");
 
+if (LangStandard::getLangStandardForKind(Opts.LangStd).isC11()) {
+  Builder.defineMacro("__STDC_NO_ATOMICS__");
+  Builder.defineMacro("__STDC_NO_THREADS__");
+}
+
 if (Opts.EnableAIXExtendedAltivecABI)
   Builder.defineMacro("__EXTABI__");
 


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -723,6 +723,25 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-NOTHREADSAFE %s
 // PPC-AIX-NOTHREADSAFE-NOT:#define _THREAD_SAFE 1
 
+// RUN: %clang_cc1 -x c -std=c11 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=c1x -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=iso9899:2011 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.10.0 -fno-signed-char  < /dev/null | FileChec

[PATCH] D103663: [AMDGPU] Add gfx1013 target

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

You need to replace HasGFX10_BEncoding with HasGFX10_AEncoding in the BVH and 
IMAGE_MSAA_LOAD_X. You also need to update llvm.amdgcn.image.msaa.load.x.ll 
test to include gfx1013.




Comment at: llvm/lib/Target/AMDGPU/AMDGPU.td:1106
   [FeatureGFX10,
FeatureGFX10_BEncoding,
FeatureGFX10_3Insts,

gfx1030 should now include FeatureGFX10_AEncoding as well. 10_B is an extension 
above 10_A.



Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.intersect_ray.ll:4
+; RUN: llc -global-isel -march=amdgcn -mcpu=gfx1013 -verify-machineinstrs < %s 
| FileCheck -check-prefix=GCN %s
+; RUN: llc -global-isel -march=amdgcn -mcpu=gfx1012 -verify-machineinstrs < %s 
| FileCheck -check-prefix=GCN %s
 

foad wrote:
> This test surely should not pass for gfx1012, since it does not have these 
> instructions. And with your patch as written it should fail for gfx1013 too, 
> since they are predicated on HasGFX10_BEncoding.
> 
> @rampitec any idea what is wrong here? Apparently the backend will happily 
> generate image_bvh_intersect_ray instructions even for gfx900!
Indeed. MIMG_IntersectRay has this:

```
  let SubtargetPredicate = HasGFX10_BEncoding,
  AssemblerPredicate = HasGFX10_BEncoding,
```
but apparently SubtargetPredicate did not work. It needs to be fixed.

gfx1012 does not have it, gfx1013 does though. That is what GFX10_A encoding is 
about, 10_B it has to be replaced with 10_A in BVH and MSAA load.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103663

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

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

Remains the issue with the alignment, I would like to know @MyDeveloperDay 's 
opinion on that. Should the values be right aligned, or left aligned? As far as 
I see all alignment in clang-format is left until now.




Comment at: clang/lib/Format/WhitespaceManager.cpp:982
+void WhitespaceManager::alignArrayInitializers(unsigned Start, unsigned End) {
+
+  auto CellDescs = getCells(Start, End);

This is unneccessary. (But doesn't need a new revision until approved.)



Comment at: clang/lib/Format/WhitespaceManager.cpp:1081
+unsigned End) {
+
+  unsigned Depth = 0;

Same


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

ping


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

https://reviews.llvm.org/D98798

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

[PATCH] D103615: [Clang] Add option for vector compare compatibility.

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

As far as I can see, there is no good reason for the special treatment of 
vector bool/pixel going forward. Could we drop this special treatment, or at 
least change the default to use a scalar results across the board (consistent 
with XL's behaviour and clang's current behaviour for most cases).




Comment at: clang/include/clang/Driver/Options.td:3811
   MarshallingInfoFlag>;
+def vector_abi_compat : Joined<["-"], "vector-abi-compat=">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Determines whether vector compare returns a vector or a scalar. 
Options: default, gcc, xl.">,

I'm not sure the term "ABI" is really applicable. Maybe we should call it 
"vector-compare-compat="



Comment at: clang/test/CodeGen/vector-compat-pixel-bool-ternary.c:6
+// RUN:   -vector-abi-compat=gcc -triple powerpc-unknown-unknown -S -emit-llvm 
%s -o - 2>&1| FileCheck %s --check-prefix=ERROR
+// RUN: %clang_cc1 -target-feature +altivec -target-feature +vsx \
+// RUN:   -vector-abi-compat=xl -triple powerpc-unknown-unknown -S -emit-llvm 
%s -o - | FileCheck %s

I only see the clang FE interface being tested. Does this have to be specified 
through `-Xclang -vector-abi-compat=...` or is there a clang driver option for 
it as well? I think we should have a clang driver option and have at least one 
test for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103615

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

[PATCH] D101630: [HIP] Fix device-only compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D101630#2799472 , @tra wrote:

> In D101630#2799425 , @yaxunl wrote:
>
>> But how do we control emitting LLVM IR with or without bundle? `-emit-llvm 
>> -emit-gpu-object` or `-emit-llvm -emit-gpu-bundle`? `-emit-*` is usually for 
>> specifying a specific file type.
>
> Hmm. I forgot that HIP can bundle things other than objects. `-emit-llvm 
> -emit-gpu-bundle` looks reasonable, but `-emit-llvm -emit-gpu-object` is 
> indeed odd.
> OK. Making it some sort of do/do-not bundle flag makes sense. How about just 
> `--[no-]gpu-bundle-output`?

--[no]gpu-bundle-output sounds good.


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

https://reviews.llvm.org/D101630

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

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

@vsk Do you mind investigating macOS `Posix/instrprof-dynamic-one-shared.test` 
and `Posix/instrprof-dynamic-two-shared.test` failures on 
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8845381350758503248/+/steps/package_clang/0/stdout
 ? :) I don't have a macOS machine.

I think the only behavior change for macOS is this block:

  // Data variables can be private if not referenced by code.
  if (!DataReferencedByCode) {
Linkage = GlobalValue::PrivateLinkage;
Visibility = GlobalValue::DefaultVisibility;
  }

If not easy to fix, I'll just exclude isOSBinFormatMachO() for now...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103372

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

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 349915.
MaskRay added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Re-upload after revert


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103372

Files:
  clang/test/Profile/c-linkage-available_externally.c
  clang/test/Profile/c-linkage.c
  clang/test/Profile/cxx-linkage.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/comdat.ll
  llvm/test/Instrumentation/InstrProfiling/icall.ll
  llvm/test/Instrumentation/InstrProfiling/linkage.ll
  llvm/test/Instrumentation/InstrProfiling/platform.ll
  llvm/test/Instrumentation/InstrProfiling/profiling.ll
  llvm/test/Transforms/PGOProfile/comdat_internal.ll

Index: llvm/test/Transforms/PGOProfile/comdat_internal.ll
===
--- llvm/test/Transforms/PGOProfile/comdat_internal.ll
+++ llvm/test/Transforms/PGOProfile/comdat_internal.ll
@@ -7,16 +7,16 @@
 ; CHECK: $foo = comdat any
 
 ; CHECK: $__llvm_profile_raw_version = comdat any
-; CHECK: $__profd__stdin__foo.[[FOO_HASH:[0-9]+]] = comdat any
+; CHECK: $__profc__stdin__foo.[[#FOO_HASH:]] = comdat any
 
 @bar = global i32 ()* @foo, align 8
 
 ; CHECK: @__llvm_profile_raw_version = constant i64 {{[0-9]+}}, comdat
 ; CHECK-NOT: __profn__stdin__foo
-; CHECK: @__profc__stdin__foo.[[FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd__stdin__foo.[[FOO_HASH]]), align 8
-; CHECK: @__profd__stdin__foo.[[FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[FOO_HASH]], i32 0, i32 0), i8* null
+; CHECK: @__profc__stdin__foo.[[#FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; CHECK: @__profd__stdin__foo.[[#FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[#FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[#FOO_HASH]], i32 0, i32 0), i8* null
 ; CHECK-NOT: bitcast (i32 ()* @foo to i8*)
-; CHECK-SAME: , i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat, align 8
+; CHECK-SAME: , i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profc__stdin__foo.[[#FOO_HASH]]), align 8
 ; CHECK: @__llvm_prf_nm
 ; CHECK: @llvm.compiler.used
 
Index: llvm/test/Instrumentation/InstrProfiling/profiling.ll
===
--- llvm/test/Instrumentation/InstrProfiling/profiling.ll
+++ llvm/test/Instrumentation/InstrProfiling/profiling.ll
@@ -17,8 +17,8 @@
 @__profn_baz = private constant [3 x i8] c"baz"
 ; CHECK-NOT: __profn_baz
 
-; ELF:   @__profc_foo = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_foo), align 8
-; ELF:   @__profd_foo = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_foo = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_foo = private {{.*}}, section "__llvm_prf_data", comdat($__profc_foo), align 8
 ; MACHO: @__profc_foo = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__profd_foo = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
 ; WIN:   @__profc_foo = private global [1 x i64] zeroinitializer, section ".lprfc$M", align 8
@@ -28,8 +28,8 @@
   ret void
 }
 
-; ELF:   @__profc_bar = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_bar), align 8
-; ELF:   @__profd_bar = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_bar = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_bar = private {{.*}}, section "__llvm_prf_data", comdat($__profc_bar), align 8
 ; MACHO: @__profc_bar = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__profd_bar = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8
 ; WIN:   @__profc_bar = private global [1 x i64] zeroinitializer, section ".lprfc$M", align 8
@@ -39,8 +39,8 @@
   ret void
 }
 
-; ELF:   @__profc_baz = private global [3 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profd_baz), align 8
-; ELF:   @__profd_baz = private {{.*}}, section "__llvm_prf_data", comdat, align 8
+; ELF:   @__profc_baz = private global [3 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
+; ELF:   @__profd_baz = private {{.*}}, section "__llvm_prf_data", comdat($__profc_baz), align 8
 ; MACHO: @__profc_baz = private global [3 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 ; MACHO: @__prof

[PATCH] D101630: [HIP] Fix device-only compilation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D101630#2799425 , @yaxunl wrote:

> But how do we control emitting LLVM IR with or without bundle? `-emit-llvm 
> -emit-gpu-object` or `-emit-llvm -emit-gpu-bundle`? `-emit-*` is usually for 
> specifying a specific file type.

Hmm. I forgot that HIP can bundle things other than objects. `-emit-llvm 
-emit-gpu-bundle` looks reasonable, but `-emit-llvm -emit-gpu-object` is indeed 
odd.
OK. Making it some sort of do/do-not bundle flag makes sense. How about just 
`--[no-]gpu-bundle-output`?


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

https://reviews.llvm.org/D101630

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

[PATCH] D103587: [AIX] Transfer __TOS_AIX__ predefined macro

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan updated this revision to Diff 349913.
Jake-Egan added a comment.

Use this patch to target only __TOS_AIX__.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103587

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/test/Preprocessor/init-ppc.c


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -541,6 +541,7 @@
 // PPC-AIX:#define __SIZE_MAX__ 4294967295UL
 // PPC-AIX:#define __SIZE_TYPE__ long unsigned int
 // PPC-AIX:#define __SIZE_WIDTH__ 32
+// PPC-AIX:#define __TOS_AIX__ 1
 // PPC-AIX:#define __UINT16_C_SUFFIX__
 // PPC-AIX:#define __UINT16_MAX__ 65535
 // PPC-AIX:#define __UINT16_TYPE__ unsigned short
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -675,6 +675,7 @@
 Builder.defineMacro("_POWER");
 
 Builder.defineMacro("_AIX");
+Builder.defineMacro("__TOS_AIX__");
 
 if (Opts.EnableAIXExtendedAltivecABI)
   Builder.defineMacro("__EXTABI__");


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -541,6 +541,7 @@
 // PPC-AIX:#define __SIZE_MAX__ 4294967295UL
 // PPC-AIX:#define __SIZE_TYPE__ long unsigned int
 // PPC-AIX:#define __SIZE_WIDTH__ 32
+// PPC-AIX:#define __TOS_AIX__ 1
 // PPC-AIX:#define __UINT16_C_SUFFIX__
 // PPC-AIX:#define __UINT16_MAX__ 65535
 // PPC-AIX:#define __UINT16_TYPE__ unsigned short
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -675,6 +675,7 @@
 Builder.defineMacro("_POWER");
 
 Builder.defineMacro("_AIX");
+Builder.defineMacro("__TOS_AIX__");
 
 if (Opts.EnableAIXExtendedAltivecABI)
   Builder.defineMacro("__EXTABI__");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103184: [AArch64] handle -Wa,-march=

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers requested changes to this revision.
nickdesaulniers added inline comments.
This revision now requires changes to proceed.



Comment at: clang/test/Driver/aarch64-target-as-march.s:29
+// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
+
+// MULTIPLE-VALUES: "-target-feature" "+v8.1a

jcai19 wrote:
> DavidSpickett wrote:
> > Add a test with `-Wa,-march=armv8.2-a,-march=armv8.1-a` (one -Wa with 
> > multiple values attached), then repeat the two tests but with `-Xassembler` 
> > instead.
> I added a test case for -Xassembler -march=armv8.2-a,-march=armv8.1-a,  but 
> it seems it does not support multiple values in one invocation?
> 
> clang --target=aarch64-linux-gnueabi -Xassembler 
> -march=armv8.2-a,-march=armv8.1-a -c foo.s -o ias.o -###
> ...
> clang-13: error: the clang compiler does not support '-Xassembler 
> -march=armv8.2-a,-march=armv8.1-a'
> ...
I don't think @DavidSpickett meant more `UNUSED-WARNING` tests, but tests that 
check the actual chosen value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

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

[PATCH] D103184: [AArch64] handle -Wa,-march=

[Jian Cai via Phabricator via cfe-commits]

jcai19 added a comment.

In D103184#2798398 , @DavidSpickett 
wrote:

>> LGTM; any additional thoughts @DavidSpickett ?
>
> A couple of additional tests just to be safe but otherwise LGTM.
>
> Also please include in the commit message the "rules" that we're using here 
> for parsing. Like I did for the arm change.

Thanks all!




Comment at: clang/test/Driver/aarch64-target-as-march.s:29
+// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
+
+// MULTIPLE-VALUES: "-target-feature" "+v8.1a

DavidSpickett wrote:
> Add a test with `-Wa,-march=armv8.2-a,-march=armv8.1-a` (one -Wa with 
> multiple values attached), then repeat the two tests but with `-Xassembler` 
> instead.
I added a test case for -Xassembler -march=armv8.2-a,-march=armv8.1-a,  but it 
seems it does not support multiple values in one invocation?

clang --target=aarch64-linux-gnueabi -Xassembler 
-march=armv8.2-a,-march=armv8.1-a -c foo.s -o ias.o -###
...
clang-13: error: the clang compiler does not support '-Xassembler 
-march=armv8.2-a,-march=armv8.1-a'
...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

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

[PATCH] D103184: [AArch64] handle -Wa,-march=

[Jian Cai via Phabricator via cfe-commits]

jcai19 updated this revision to Diff 349912.
jcai19 added a comment.

Address the comments and add more tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/aarch64-target-as-march.s

Index: clang/test/Driver/aarch64-target-as-march.s
===
--- /dev/null
+++ clang/test/Driver/aarch64-target-as-march.s
@@ -0,0 +1,48 @@
+/// These tests make sure that options passed to the assembler
+/// via -Wa or -Xassembler are applied correctly to assembler inputs.
+
+/// Does not apply to non assembly files
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.1-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-1 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.1-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-1 %s
+
+// TARGET-FEATURE-1-NOT: "-target-feature" "+v8.1a"
+
+/// Does apply to assembler input
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-2 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-2 %s
+
+// TARGET-FEATURE-2: "-target-feature" "+v8.2a"
+
+/// No unused argument warnings when there are multiple values
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.1-a -Wa,-march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=UNUSED-WARNING %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.2-a,-march=armv8.1-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=UNUSED-WARNING %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.1-a -Xassembler -march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=UNUSED-WARNING %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.2-a,-march=armv8.1-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=UNUSED-WARNING %s
+
+// UNUSED-WARNING-NOT: warning: argument unused during compilation
+
+/// Last march to assembler wins
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.2-a -Wa,-march=armv8.1-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
+
+// MULTIPLE-VALUES: "-target-feature" "+v8.1a
+// MULTIPLE-VALUES-NOT: "-target-feature" "+v8.2a
+
+/// march to compiler and assembler, we choose the one suited to the input file type
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-3 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-4 %s
+
+// TARGET-FEATURE-3: "-target-feature" "+v8.3a"
+// TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
+// TARGET-FEATURE-4: "-target-feature" "+v8.4a"
+// TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -344,7 +344,7 @@
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_32:
   case llvm::Triple::aarch64_be:
-aarch64::getAArch64TargetFeatures(D, Triple, Args, Features);
+aarch64::getAArch64TargetFeatures(D, Triple, Args, Features, ForAS);
 break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
Index: clang/lib/Driver/ToolChains/Arch/AArch64.h
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.h
+++ clang/lib/Driver/ToolChains/Arch/AArch64.h
@@ -22,7 +22,8 @@
 
 void getAArch64TargetFeatures(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args,
-  std::vector &Features);
+  std::vector &Features,
+  bool ForAS);
 
 std::string getAArch64TargetCPU(const llvm::opt::ArgList &Args,
 const llvm::Triple &Triple, llvm::opt::Arg *&A);
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -185,12 +185,20 @@
 void aarch64::getAArch64TargetFeatures(const Driver &D,
const llvm::Triple &Triple,
const ArgList &Args,
-   std::vector &Features) {
+  

[PATCH] D103097: Add DWARF address spaces mapping for SPIR

[Stuart Brady via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG333987b04589: [OpenCL] Add DWARF address spaces mapping for 
SPIR (authored by jzzheng22, committed by stuart).

Changed prior to commit:
  https://reviews.llvm.org/D103097?vs=348910&id=349910#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103097

Files:
  clang/lib/Basic/Targets/SPIR.h
  clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl


Index: clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -cl-std=CL2.0 -debug-info-kind=limited -dwarf-version=5 
-emit-llvm -O0 -triple spir-unknown-unknown -o - %s | FileCheck %s
+// RUN: %clang_cc1 -cl-std=CL2.0 -debug-info-kind=limited -dwarf-version=5 
-emit-llvm -O0 -triple spir64-unknown-unknown -o - %s | FileCheck %s
+
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_GLOBAL:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 1)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_CONSTANT:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 2)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_LOCAL:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 3)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_PRIVATE:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 0)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_GENERIC:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 4)
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar0", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_GLOBAL]], 
isLocal: false, isDefinition: true)
+global int *FileVar0;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar1", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_CONSTANT]], 
isLocal: false, isDefinition: true)
+constant int *FileVar1;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar2", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_LOCAL]], 
isLocal: false, isDefinition: true)
+local int *FileVar2;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar3", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_PRIVATE]], 
isLocal: false, isDefinition: true)
+private int *FileVar3;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar4", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_GENERIC]], 
isLocal: false, isDefinition: true)
+int *FileVar4;
Index: clang/lib/Basic/Targets/SPIR.h
===
--- clang/lib/Basic/Targets/SPIR.h
+++ clang/lib/Basic/Targets/SPIR.h
@@ -117,6 +117,11 @@
 return TargetInfo::VoidPtrBuiltinVaList;
   }
 
+  Optional
+  getDWARFAddressSpace(unsigned AddressSpace) const override {
+return AddressSpace;
+  }
+
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override 
{
 return (CC == CC_SpirFunction || CC == CC_OpenCLKernel) ? CCCR_OK
 : CCCR_Warning;


Index: clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -cl-std=CL2.0 -debug-info-kind=limited -dwarf-version=5 -emit-llvm -O0 -triple spir-unknown-unknown -o - %s | FileCheck %s
+// RUN: %clang_cc1 -cl-std=CL2.0 -debug-info-kind=limited -dwarf-version=5 -emit-llvm -O0 -triple spir64-unknown-unknown -o - %s | FileCheck %s
+
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_GLOBAL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, dwarfAddressSpace: 1)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_CONSTANT:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, dwarfAddressSpace: 2)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_LOCAL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, dwarfAddressSpace: 3)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_PRIVATE:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, dwarfAddressSpace: 0)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_GENERIC:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, dwarfAddressSpace: 4)
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar0", scope: !{{[0-9]+}}, file: !{{

[clang] 333987b - [OpenCL] Add DWARF address spaces mapping for SPIR

[Stuart Brady via cfe-commits]

Author: Jason Zheng
Date: 2021-06-04T18:10:54+01:00
New Revision: 333987b0458926332e9a1f96869ef47da25fa9b1

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

LOG: [OpenCL] Add DWARF address spaces mapping for SPIR

Extend debug info handling by adding DWARF address space mapping for
SPIR, with corresponding test case.

Reviewed By: Anastasia

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

Added: 
clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl

Modified: 
clang/lib/Basic/Targets/SPIR.h

Removed: 




diff  --git a/clang/lib/Basic/Targets/SPIR.h b/clang/lib/Basic/Targets/SPIR.h
index 638071d0cdce0..c429b27709ecb 100644
--- a/clang/lib/Basic/Targets/SPIR.h
+++ b/clang/lib/Basic/Targets/SPIR.h
@@ -117,6 +117,11 @@ class LLVM_LIBRARY_VISIBILITY SPIRTargetInfo : public 
TargetInfo {
 return TargetInfo::VoidPtrBuiltinVaList;
   }
 
+  Optional
+  getDWARFAddressSpace(unsigned AddressSpace) const override {
+return AddressSpace;
+  }
+
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override 
{
 return (CC == CC_SpirFunction || CC == CC_OpenCLKernel) ? CCCR_OK
 : CCCR_Warning;

diff  --git a/clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl 
b/clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl
new file mode 100644
index 0..28b6c674c8ffd
--- /dev/null
+++ b/clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -cl-std=CL2.0 -debug-info-kind=limited -dwarf-version=5 
-emit-llvm -O0 -triple spir-unknown-unknown -o - %s | FileCheck %s
+// RUN: %clang_cc1 -cl-std=CL2.0 -debug-info-kind=limited -dwarf-version=5 
-emit-llvm -O0 -triple spir64-unknown-unknown -o - %s | FileCheck %s
+
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_GLOBAL:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 1)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_CONSTANT:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 2)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_LOCAL:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 3)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_PRIVATE:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 0)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_GENERIC:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 4)
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar0", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_GLOBAL]], 
isLocal: false, isDefinition: true)
+global int *FileVar0;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar1", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_CONSTANT]], 
isLocal: false, isDefinition: true)
+constant int *FileVar1;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar2", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_LOCAL]], 
isLocal: false, isDefinition: true)
+local int *FileVar2;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar3", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_PRIVATE]], 
isLocal: false, isDefinition: true)
+private int *FileVar3;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar4", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_GENERIC]], 
isLocal: false, isDefinition: true)
+int *FileVar4;



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

[PATCH] D103658: CUDA/HIP: Change device-use-host-var.cu's NOT "external" check to include "addrspace"

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4d9f8527dbfb: CUDA/HIP: Change device-use-host-var.cu's 
NOT "external" check to include… (authored by kzhuravl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103658

Files:
  clang/test/CodeGenCUDA/device-use-host-var.cu


Index: clang/test/CodeGenCUDA/device-use-host-var.cu
===
--- clang/test/CodeGenCUDA/device-use-host-var.cu
+++ clang/test/CodeGenCUDA/device-use-host-var.cu
@@ -65,7 +65,7 @@
 // NEG-NOT: @_ZN1BIiE1yE
 // NEG-NOT: @_Z1bIdE
 // NEG-NOT: @_ZL13var_host_only
-// NEG-NOT: external
+// NEG-NOT: {{^}}@{{.*}} = external
 
 // CHECK-LABEL: define{{.*}}@_Z7dev_funPiPPKi
 // CHECK: store i32 1


Index: clang/test/CodeGenCUDA/device-use-host-var.cu
===
--- clang/test/CodeGenCUDA/device-use-host-var.cu
+++ clang/test/CodeGenCUDA/device-use-host-var.cu
@@ -65,7 +65,7 @@
 // NEG-NOT: @_ZN1BIiE1yE
 // NEG-NOT: @_Z1bIdE
 // NEG-NOT: @_ZL13var_host_only
-// NEG-NOT: external
+// NEG-NOT: {{^}}@{{.*}} = external
 
 // CHECK-LABEL: define{{.*}}@_Z7dev_funPiPPKi
 // CHECK: store i32 1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 4d9f852 - CUDA/HIP: Change device-use-host-var.cu's NOT "external" check to include variable name

[Konstantin Zhuravlyov via cfe-commits]

Author: Konstantin Zhuravlyov
Date: 2021-06-04T13:10:00-04:00
New Revision: 4d9f8527dbfbc998baf35eec868c9dec1f8d1224

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

LOG: CUDA/HIP: Change device-use-host-var.cu's NOT "external" check to include 
variable name

Otherwise it is causing one of our build jobs to fail,
it is using "external" as directory, and NOT is
failing because "external" is found in ModuleID.

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

Added: 


Modified: 
clang/test/CodeGenCUDA/device-use-host-var.cu

Removed: 




diff  --git a/clang/test/CodeGenCUDA/device-use-host-var.cu 
b/clang/test/CodeGenCUDA/device-use-host-var.cu
index 1a504280e8488..4d3f60c2e83c7 100644
--- a/clang/test/CodeGenCUDA/device-use-host-var.cu
+++ b/clang/test/CodeGenCUDA/device-use-host-var.cu
@@ -65,7 +65,7 @@ const int var_host_only = 7;
 // NEG-NOT: @_ZN1BIiE1yE
 // NEG-NOT: @_Z1bIdE
 // NEG-NOT: @_ZL13var_host_only
-// NEG-NOT: external
+// NEG-NOT: {{^}}@{{.*}} = external
 
 // CHECK-LABEL: define{{.*}}@_Z7dev_funPiPPKi
 // CHECK: store i32 1



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

[PATCH] D101630: [HIP] Fix device-only compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D101630#2799409 , @tra wrote:

> In D101630#2798975 , @yaxunl wrote:
>
>> For sure we will need -fgpu-bundle-device-output to control bundling of 
>> intermediate files. Then adding -emit-gpu-object and -emit-gpu-bundle may be 
>> redundant and can cause confusion. What if users specify `-c 
>> -fgpu-bundle-device-output -emit-gpu-object` or `-c 
>> -fno-gpu-bundle-device-output -emit-gpu-bundle`? To me a single option 
>> -fgpu-bundle-device-output to control all device output seems cleaner.
>
> The idea is to use `-emit-gpu-object` and `-emit-gpu-bundle` instead of the  
> `-f[no-]gpu-bundle-device-output`. Otherwise they'd do exactly the same thing.
>
> I think `-emit-gpu-{object,bundle}` has a minor edge over 
> `-f[no-]gpu-bundle-device-output` as it's similar to other -emit options for 
> controlling clang compilation phases (and that's what we want to do here), 
> while `-f` options are usually for tweaking code generation.

But how do we control emitting LLVM IR with or without bundle? `-emit-llvm 
-emit-gpu-object` or `-emit-llvm -emit-gpu-bundle`? `-emit-*` is usually for 
specifying a specific file type.


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

https://reviews.llvm.org/D101630

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

[PATCH] D103040: Print default template argument if manually specified in typedef declaration.

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall 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/D103040/new/

https://reviews.llvm.org/D103040

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

[PATCH] D101630: [HIP] Fix device-only compilation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D101630#2798975 , @yaxunl wrote:

> For sure we will need -fgpu-bundle-device-output to control bundling of 
> intermediate files. Then adding -emit-gpu-object and -emit-gpu-bundle may be 
> redundant and can cause confusion. What if users specify `-c 
> -fgpu-bundle-device-output -emit-gpu-object` or `-c 
> -fno-gpu-bundle-device-output -emit-gpu-bundle`? To me a single option 
> -fgpu-bundle-device-output to control all device output seems cleaner.

The idea is to use `-emit-gpu-object` and `-emit-gpu-bundle` instead of the  
`-f[no-]gpu-bundle-device-output`. Otherwise they'd do exactly the same thing.

I think `-emit-gpu-{object,bundle}` has a minor edge over 
`-f[no-]gpu-bundle-device-output` as it's similar to other -emit options for 
controlling clang compilation phases (and that's what we want to do here), 
while `-f` options are usually for tweaking code generation.


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

https://reviews.llvm.org/D101630

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

[PATCH] D103611: Correct the behavior of va_arg checking in C++

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D103611

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

[PATCH] D102822: [Clang][CodeGen] Set the size of llvm.lifetime to unknown for scalable types.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/test/CodeGen/RISCV/riscv-v-lifetime.cpp:3
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -std=c++11 -triple riscv64 -target-feature +experimental-v \
+// RUN:   -emit-llvm -O1 -o - %s | FileCheck %s

sdesmalen wrote:
> craig.topper wrote:
> > Probably best to add -disable-llvm-passes so we don't run the optimizer.
> nit: Does `-O1` still have any effect if `-disable-llvm-passes` is also set?
Lifetime markers aren't emitted with -O0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102822

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks, Akira.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[clang] b109172 - [clang] use a different name for generated test cdb

[Mikhail Goncharov via cfe-commits]

Author: Mikhail Goncharov
Date: 2021-06-04T18:12:58+02:00
New Revision: b109172d993edacd9853a8bbb8128a94da014399

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

LOG: [clang] use a different name for generated test cdb

if build system copied source files as readonly, then override of db_tu.json
will fail

Added: 


Modified: 
clang/test/ClangScanDeps/modules-pch.c

Removed: 




diff  --git a/clang/test/ClangScanDeps/modules-pch.c 
b/clang/test/ClangScanDeps/modules-pch.c
index 44b1a0df764c..ddb6949f5e6f 100644
--- a/clang/test/ClangScanDeps/modules-pch.c
+++ b/clang/test/ClangScanDeps/modules-pch.c
@@ -8,6 +8,6 @@
 
 // Scan dependencies of the TU:
 //
-// RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_tu.json > %t/cdb_tu.json
-// RUN: clang-scan-deps -compilation-database %t/cdb_tu.json -format 
experimental-full \
+// RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_tu.json > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format 
experimental-full \
 // RUN:   -generate-modules-path-args -module-files-dir %t/build



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

[PATCH] D103538: [clangd] Run code completion on each token coverd by --check-lines

[Adam Czachorowski via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeba3ee04d450: [clangd] Run code completion on each token 
coverd by --check-lines (authored by adamcz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103538

Files:
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -62,7 +62,8 @@
 // Implemented in Check.cpp.
 bool check(const llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts);
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   bool EnableCodeCompletion);
 
 namespace {
 
@@ -929,7 +930,11 @@
   uint32_t Line = Pos.line + 1; // Position::line is 0-based.
   return Line >= Begin && Line <= End;
 };
-return check(Path, ShouldCheckLine, TFS, Opts)
+// For now code completion is enabled any time the range is limited via
+// --check-lines. If it turns out to be to slow, we can introduce a
+// dedicated flag for that instead.
+return check(Path, ShouldCheckLine, TFS, Opts,
+ /*EnableCodeCompletion=*/!CheckFileLines.empty())
? 0
: static_cast(ErrorResultCode::CheckFailed);
   }
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -193,10 +193,15 @@
 
   // Run AST-based features at each token in the file.
   void testLocationFeatures(
-  llvm::function_ref ShouldCheckLine) {
+  llvm::function_ref ShouldCheckLine,
+  const bool EnableCodeCompletion) {
 log("Testing features at each token (may be slow in large files)");
 auto &SM = AST->getSourceManager();
 auto SpelledTokens = AST->getTokens().spelledTokens(SM.getMainFileID());
+
+CodeCompleteOptions CCOpts = Opts.CodeComplete;
+CCOpts.Index = &Index;
+
 for (const auto &Tok : SpelledTokens) {
   unsigned Start = AST->getSourceManager().getFileOffset(Tok.location());
   unsigned End = Start + Tok.length();
@@ -233,8 +238,12 @@
   auto Hover = getHover(*AST, Pos, Style, &Index);
   vlog("hover: {0}", Hover.hasValue());
 
-  // FIXME: it'd be nice to include code completion, but it's too slow.
-  // Maybe in combination with a line restriction?
+  if (EnableCodeCompletion) {
+Position EndPos = offsetToPosition(Inputs.Contents, End);
+auto CC = codeComplete(File, EndPos, Preamble.get(), Inputs, CCOpts);
+vlog("code completion: {0}",
+ CC.Completions.empty() ? "" : CC.Completions[0].Name);
+  }
 }
   }
 };
@@ -243,7 +252,8 @@
 
 bool check(llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts) {
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   bool EnableCodeCompletion) {
   llvm::SmallString<0> FakeFile;
   llvm::Optional Contents;
   if (File.empty()) {
@@ -267,7 +277,7 @@
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
   !C.buildAST())
 return false;
-  C.testLocationFeatures(ShouldCheckLine);
+  C.testLocationFeatures(ShouldCheckLine, EnableCodeCompletion);
 
   log("All checks completed, {0} errors", C.ErrCount);
   return C.ErrCount == 0;


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -62,7 +62,8 @@
 // Implemented in Check.cpp.
 bool check(const llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts);
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   bool EnableCodeCompletion);
 
 namespace {
 
@@ -929,7 +930,11 @@
   uint32_t Line = Pos.line + 1; // Position::line is 0-based.
   return Line >= Begin && Line <= End;
 };
-return check(Path, ShouldCheckLine, TFS, Opts)
+// For now code completion is enabled any time the range is limited via
+// --check-lines. If it turns out to be to slow, we can introduce a
+// dedicated flag for that instead.
+return check(Path, ShouldCheckLine, TFS, Opts,
+ /*EnableCodeCompletion=*/!CheckFileLines.empty())
? 0
: static_cast(ErrorResultCode::CheckFailed);
   }
Index: clang-tools-extra/clangd/tool/Check.cpp

[clang-tools-extra] eba3ee0 - [clangd] Run code completion on each token coverd by --check-lines

[Adam Czachorowski via cfe-commits]

Author: Adam Czachorowski
Date: 2021-06-04T17:51:42+02:00
New Revision: eba3ee04d450230f7ac1f88b1abd7b09c600c82d

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

LOG: [clangd] Run code completion on each token coverd by --check-lines

In --check mode we do not run code completion because it is too slow,
especially on larger files. With the introducation of --check-lines we
can narrow down the scope and thus we can afford to do code completion.

We vlog() the top completion result, but that's not really the point.
The most value will come from being able to reproduce crashes that occur
during code completion and require preamble build or index (and thus are
more difficult to reproduce with -code-complete-at).

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

Added: 


Modified: 
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/tool/Check.cpp 
b/clang-tools-extra/clangd/tool/Check.cpp
index 89487bd8607f1..88eb945d20d43 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -193,10 +193,15 @@ class Checker {
 
   // Run AST-based features at each token in the file.
   void testLocationFeatures(
-  llvm::function_ref ShouldCheckLine) {
+  llvm::function_ref ShouldCheckLine,
+  const bool EnableCodeCompletion) {
 log("Testing features at each token (may be slow in large files)");
 auto &SM = AST->getSourceManager();
 auto SpelledTokens = AST->getTokens().spelledTokens(SM.getMainFileID());
+
+CodeCompleteOptions CCOpts = Opts.CodeComplete;
+CCOpts.Index = &Index;
+
 for (const auto &Tok : SpelledTokens) {
   unsigned Start = AST->getSourceManager().getFileOffset(Tok.location());
   unsigned End = Start + Tok.length();
@@ -233,8 +238,12 @@ class Checker {
   auto Hover = getHover(*AST, Pos, Style, &Index);
   vlog("hover: {0}", Hover.hasValue());
 
-  // FIXME: it'd be nice to include code completion, but it's too slow.
-  // Maybe in combination with a line restriction?
+  if (EnableCodeCompletion) {
+Position EndPos = offsetToPosition(Inputs.Contents, End);
+auto CC = codeComplete(File, EndPos, Preamble.get(), Inputs, CCOpts);
+vlog("code completion: {0}",
+ CC.Completions.empty() ? "" : CC.Completions[0].Name);
+  }
 }
   }
 };
@@ -243,7 +252,8 @@ class Checker {
 
 bool check(llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts) {
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   bool EnableCodeCompletion) {
   llvm::SmallString<0> FakeFile;
   llvm::Optional Contents;
   if (File.empty()) {
@@ -267,7 +277,7 @@ bool check(llvm::StringRef File,
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
   !C.buildAST())
 return false;
-  C.testLocationFeatures(ShouldCheckLine);
+  C.testLocationFeatures(ShouldCheckLine, EnableCodeCompletion);
 
   log("All checks completed, {0} errors", C.ErrCount);
   return C.ErrCount == 0;

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index f0aa89b8091fb..0f9e5c89e42c8 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -62,7 +62,8 @@ namespace clangd {
 // Implemented in Check.cpp.
 bool check(const llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts);
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   bool EnableCodeCompletion);
 
 namespace {
 
@@ -929,7 +930,11 @@ clangd accepts flags on the commandline, and in the 
CLANGD_FLAGS environment var
   uint32_t Line = Pos.line + 1; // Position::line is 0-based.
   return Line >= Begin && Line <= End;
 };
-return check(Path, ShouldCheckLine, TFS, Opts)
+// For now code completion is enabled any time the range is limited via
+// --check-lines. If it turns out to be to slow, we can introduce a
+// dedicated flag for that instead.
+return check(Path, ShouldCheckLine, TFS, Opts,
+ /*EnableCodeCompletion=*/!CheckFileLines.empty())
? 0
: static_cast(ErrorResultCode::CheckFailed);
   }



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

[PATCH] D97699: [analyzer] Add InvalidPtrChecker

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

I have not too deep knowledge about checker development but still found 
(hopefully valid) issues, even if only a part of them.




Comment at: clang/docs/analyzer/checkers.rst:2117
+// envp may no longer point to the current environment
+// this program has unanticipated behavior, since envp
+// does not reflect changes made by setenv function.

From my understanding the "unanticipated behavior" here is clearly a bug: We do 
not know if the `envp` points to any valid location. It is not guaranteed that 
the original value is preserved (in this case it would have sense to use it). 
And it is not known if it will point to the new and correct array. Unless we 
know how the internal implementation exactly works.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:947
+
+} // end "alpha.cert.env"
+

I have multiple issues with the documentation texts but they look not final 
anyway. And the package and checker names could be better. There are already 
checkers for CERT rules that do not reside in the cert package, it is not good 
to have some checkers in a `cert` package and some not. It is likely that 
checkers will check for more rules or parts of the rules. Checker aliases are a 
better solution for the cert checker names. (And now the `cert` would contain 
checker with name not matching a rule: `InvalidPtr`.) The name `InvalidPtr` 
sounds too general, even if in an `env˙ package.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:28
+// pointer. These functions include: getenv, localeconv, asctime, setlocale,
+// strerror
+//===--===//

I think we do not need to repeat the documentation here.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:180
+// Note: This pointer has type 'const MemRegion *'
+REGISTER_TRAIT_WITH_PROGRAMSTATE(EnvPtrRegion, const void *)
+

Is it not possible to use `const MemRegion *` then?



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:192
+  return false;
+}
+

`evalCall` should be used only if the called function is exclusively modeled by 
this checker which is not the case here. The `checkPostCall` should be 
sufficient instead of `evalCall`.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:227
+  if (const auto *Reg =
+  State->get(FunctionName.data())) {
+const auto *PrevReg = *Reg;

`auto` is not to be used if the type is not clearly visible in the context. And 
this would be exactly `auto **` here. (But better is `const MemRegion **`.) 
Makes the later statement (assign to `PrevReg`) "messy" (and `auto` is bad 
there too).



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:227
+  if (const auto *Reg =
+  State->get(FunctionName.data())) {
+const auto *PrevReg = *Reg;

balazske wrote:
> `auto` is not to be used if the type is not clearly visible in the context. 
> And this would be exactly `auto **` here. (But better is `const MemRegion 
> **`.) Makes the later statement (assign to `PrevReg`) "messy" (and `auto` is 
> bad there too).
`.data` is unnecessary here?



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:319
+C.emitReport(std::move(Report));
+C.addTransition(State);
+  }

`addTransition` is not needed here, the state was not modified.
Save the error node returned by `generateNonFatalErrorNode` and pass it to the 
next call of `generateNonFatalErrorNode` as the `Pred` parameter.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:341
+  State = State->set(
+  reinterpret_cast(const_cast(EnvpReg)));
+  C.addTransition(State);

Are these casts really needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97699

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

[PATCH] D103097: Add DWARF address spaces mapping for SPIR

[Stuart Brady via Phabricator via cfe-commits]

stuart added inline comments.



Comment at: clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl:22
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar5", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_GLOBAL]], 
isLocal: false, isDefinition: true)
+global int *global FileVar5;
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar6", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_CONSTANT]], 
isLocal: false, isDefinition: true)

Anastasia wrote:
> btw this variable is a duplicate of `FileVar0` for your change. In clang 
> parser:
> `global int *ptr;`
> is the same as 
> `global int *global ptr;`
> 
> 
> The same applies to local variables of `Type *` and `Type *private` as they 
> are equivalent on AST level too.
> 
> This is due to the address space inference rules if you are interested in 
> more details: 
> https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_C.html#addr-spaces-inference
> 
> Perhaps you could reduce the test case a bit by removing the duplicate 
> testing?
> 
> Also I would suggest separating with empty lines every variable declaration 
> with its corresponding CHECK line to improve the readability.
> 
> 
> ```
> CHECK: <...>
> Type var1;
> 
> CHECK: <...>
> Type var2;
> ```
> 
In case this review feeds into changes made for other test files, it may be 
worth noting that the test in question uses OpenCL C 2.0 (`-cl-std=CL2.0`), and 
therefore the generic address space as the default in many contexts, rather 
than `private`. (This comment is made not for direct benefit for this review 
itself, but for the benefit of anyone who may be reading who is not already 
especially familiar with OpenCL address spaces.)

The duplicated testing has now been removed from the newly added test, though.


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

https://reviews.llvm.org/D103097

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

[PATCH] D99459: [OpenMP] Implement '#pragma omp unroll'.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:12821
+/// Determine whether an expression is constant without emitting diagnostic.
+static bool isConstantExpression(Sema &SemaRef, Expr *E) {
+  struct ConstTripcountDiagnoser : public Sema::VerifyICEDiagnoser {

Meinersbur wrote:
> ABataev wrote:
> > Can you use `VerifyPositiveIntegerConstantInClause` function instead?
> The reason for this new function is that 
> `VerifyPositiveIntegerConstantInClause` also emits a note on why it is not a 
> constant expression:
> ```
> note: read of non-const variable '.capture_expr.' is not allowed in a 
> constant expression
> ```
> Printing internal variable names is more confusing than helpful to the user.
Maybe expand `VerifyPositiveIntegerConstantInClause` to exclude this rather 
than add a new function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99459

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

[PATCH] D103612: [flang][driver] Add `-fno-unparse-typed-exprs`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 349881.
awarzynski added a comment.

Fix build and test failure


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103612

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/FrontendOptions.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Driver/unparse-typed-exprs.f95
  flang/tools/f18/f18.cpp
  flang/tools/f18/flang

Index: flang/tools/f18/flang
===
--- flang/tools/f18/flang
+++ flang/tools/f18/flang
@@ -8,7 +8,7 @@
 #======#
 
 wd=$(cd $(dirname "$0")/.. && pwd)
-opts="-module-suffix .f18.mod "
+opts="-fno-unparse-typed-exprs -module-suffix .f18.mod "
 if ! $wd/bin/f18 $opts "$@"
 then status=$?
  echo flang: in $PWD, f18 failed with exit status $status: $wd/bin/f18 $opts "$@" >&2
Index: flang/tools/f18/f18.cpp
===
--- flang/tools/f18/f18.cpp
+++ flang/tools/f18/f18.cpp
@@ -105,7 +105,7 @@
   bool debugModuleWriter{false};
   bool defaultReal8{false};
   bool measureTree{false};
-  bool unparseTypedExprsToF18_FC{false};
+  bool unparseTypedExprs{true};
   std::vector F18_FCArgs;
   const char *prefix{nullptr};
   bool getDefinition{false};
@@ -322,7 +322,8 @@
 Unparse(llvm::outs(), parseTree, driver.encoding, true /*capitalize*/,
 options.features.IsEnabled(
 Fortran::common::LanguageFeature::BackslashEscapes),
-nullptr /* action before each statement */, &asFortran);
+nullptr /* action before each statement */,
+driver.unparseTypedExprs ? &asFortran : nullptr);
 return {};
   }
   if (driver.dumpPreFirTree) {
@@ -353,7 +354,7 @@
 options.features.IsEnabled(
 Fortran::common::LanguageFeature::BackslashEscapes),
 nullptr /* action before each statement */,
-driver.unparseTypedExprsToF18_FC ? &asFortran : nullptr);
+driver.unparseTypedExprs ? &asFortran : nullptr);
   }
 
   RunOtherCompiler(driver, tmpSourcePath.data(), relo.data());
@@ -578,8 +579,8 @@
 } else if (arg == "-funparse-with-symbols" ||
 arg == "-fdebug-unparse-with-symbols") {
   driver.dumpUnparseWithSymbols = true;
-} else if (arg == "-funparse-typed-exprs-to-f18-fc") {
-  driver.unparseTypedExprsToF18_FC = true;
+} else if (arg == "-fno-unparse-typed-exprs") {
+  driver.unparseTypedExprs = false;
 } else if (arg == "-fparse-only" || arg == "-fsyntax-only") {
   driver.syntaxOnly = true;
 } else if (arg == "-c") {
Index: flang/test/Driver/unparse-typed-exprs.f95
===
--- /dev/null
+++ flang/test/Driver/unparse-typed-exprs.f95
@@ -0,0 +1,34 @@
+! Tests `-fno-unparse-typed-exprs-to-f18-fc` frontend option
+
+!--
+! RUN lines
+!--
+! RUN: %flang_fc1 -fdebug-unparse  %s | FileCheck %s --check-prefix=UNPARSED_TYPED_EXPR
+! RUN: %flang_fc1 -fdebug-unparse -fno-unparse-typed-exprs %s | FileCheck %s --check-prefix=AS_FORTRAN
+
+!--
+! EXPECTED OUTPUT: default
+!--
+! UNPARSED_TYPED_EXPR: PROGRAM test_allocated
+! UNPARSED_TYPED_EXPR-NEXT:  INTEGER :: i = 4_4
+! UNPARSED_TYPED_EXPR-NEXT:  REAL(KIND=4_4), ALLOCATABLE :: x(:)
+! UNPARSED_TYPED_EXPR-NEXT:  IF (.NOT.allocated(x)) ALLOCATE(x(i))
+! UNPARSED_TYPED_EXPR-NEXT: END PROGRAM test_allocated
+
+!-
+! EXPECTED OUTPUT: unparsed as Fortran
+!--
+! AS_FORTRAN: PROGRAM test_allocated
+! AS_FORTRAN-NEXT:  INTEGER :: i = 4
+! AS_FORTRAN-NEXT:  REAL(KIND=4), ALLOCATABLE :: x(:)
+! AS_FORTRAN-NEXT:  IF (.NOT.allocated(x)) ALLOCATE(x(i))
+! AS_FORTRAN-NEXT: END PROGRAM test_allocated
+
+!--
+! INPUT
+!--
+program test_allocated
+  integer :: i = 4
+  real(4), allocatable :: x(:)
+  if (.not. allocated(x)) allocate(x(i))
+end program test_allocated
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -100,6 +100,8 @@
 ! HELP-FC1-NEXT:Specify where to find the compiled intrinsic modules
 ! HELP-FC1-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-FC1-NEXT: -flogical-abbreviations Enable logical abbreviations
+! HELP-FC1-NEXT: -fno-unparse-typed-exprs
+! HELP-FC1-NEXT:Don't unparse typed expressions
 ! HELP-FC1-NEXT: -fopenacc  Enable OpenACC
 ! HELP-FC1-NEXT: -fopenmp 

[PATCH] D99540: [clangd] Preserve diags between tweak enumeration and execution

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

> Sorry I've lost my context - did we decide to move forward with this patch?

I don't think we've came to a conclusion, just decided to postpone until 
needed. I believe the `cases` design is really a good fit for making tweaks 
expose multiple code actions.

But we've actually missed one thing while discussing this patch. It actually 
allows data from `clang::Diagnostic` to be stashed into `clangd::Diag` for use 
later on. Modules can actually stash this info while the AST is being built, 
later on they can be retrieved directly from the `ParsedAST::getDiagnostics()`. 
But this creates the N*M problem again, and feels like a hack.
What we can do instead is during `enumerateTweak`, we can group `data` in 
diagnostics (making sure `data` stored in `diagnostic` is keyed by `tweak::id`) 
and pass an additional array of json objects in `tweak::prepare`.  This will 
make the problem N+M again and make the data passing explicit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99540

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

[PATCH] D97085: [OpenMP] libomp: implement OpenMP 5.1 inoutset task dependence type

[Jonathan Peyton via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D97085

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

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

wco




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7155
-if (EH.Asynch)
-  CmdArgs.push_back("-fasync-exceptions");
   }

Not really sure I understand this change. Isn't the case that if I compile with 
-EHa, I want the -fasync-exceptions option to be added to my options?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103664

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

[PATCH] D103538: [clangd] Run code completion on each token coverd by --check-lines

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz updated this revision to Diff 349878.
adamcz added a comment.

review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103538

Files:
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -62,7 +62,8 @@
 // Implemented in Check.cpp.
 bool check(const llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts);
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   bool EnableCodeCompletion);
 
 namespace {
 
@@ -929,7 +930,11 @@
   uint32_t Line = Pos.line + 1; // Position::line is 0-based.
   return Line >= Begin && Line <= End;
 };
-return check(Path, ShouldCheckLine, TFS, Opts)
+// For now code completion is enabled any time the range is limited via
+// --check-lines. If it turns out to be to slow, we can introduce a
+// dedicated flag for that instead.
+return check(Path, ShouldCheckLine, TFS, Opts,
+ /*EnableCodeCompletion=*/!CheckFileLines.empty())
? 0
: static_cast(ErrorResultCode::CheckFailed);
   }
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -193,10 +193,15 @@
 
   // Run AST-based features at each token in the file.
   void testLocationFeatures(
-  llvm::function_ref ShouldCheckLine) {
+  llvm::function_ref ShouldCheckLine,
+  const bool EnableCodeCompletion) {
 log("Testing features at each token (may be slow in large files)");
 auto &SM = AST->getSourceManager();
 auto SpelledTokens = AST->getTokens().spelledTokens(SM.getMainFileID());
+
+CodeCompleteOptions CCOpts = Opts.CodeComplete;
+CCOpts.Index = &Index;
+
 for (const auto &Tok : SpelledTokens) {
   unsigned Start = AST->getSourceManager().getFileOffset(Tok.location());
   unsigned End = Start + Tok.length();
@@ -228,8 +233,12 @@
   auto Hover = getHover(*AST, Pos, Style, &Index);
   vlog("hover: {0}", Hover.hasValue());
 
-  // FIXME: it'd be nice to include code completion, but it's too slow.
-  // Maybe in combination with a line restriction?
+  if (EnableCodeCompletion) {
+Position EndPos = offsetToPosition(Inputs.Contents, End);
+auto CC = codeComplete(File, EndPos, Preamble.get(), Inputs, CCOpts);
+vlog("code completion: {0}",
+ CC.Completions.empty() ? "" : CC.Completions[0].Name);
+  }
 }
   }
 };
@@ -238,7 +247,8 @@
 
 bool check(llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts) {
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   bool EnableCodeCompletion) {
   llvm::SmallString<0> FakeFile;
   llvm::Optional Contents;
   if (File.empty()) {
@@ -262,7 +272,7 @@
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
   !C.buildAST())
 return false;
-  C.testLocationFeatures(ShouldCheckLine);
+  C.testLocationFeatures(ShouldCheckLine, EnableCodeCompletion);
 
   log("All checks completed, {0} errors", C.ErrCount);
   return C.ErrCount == 0;


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -62,7 +62,8 @@
 // Implemented in Check.cpp.
 bool check(const llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts);
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   bool EnableCodeCompletion);
 
 namespace {
 
@@ -929,7 +930,11 @@
   uint32_t Line = Pos.line + 1; // Position::line is 0-based.
   return Line >= Begin && Line <= End;
 };
-return check(Path, ShouldCheckLine, TFS, Opts)
+// For now code completion is enabled any time the range is limited via
+// --check-lines. If it turns out to be to slow, we can introduce a
+// dedicated flag for that instead.
+return check(Path, ShouldCheckLine, TFS, Opts,
+ /*EnableCodeCompletion=*/!CheckFileLines.empty())
? 0
: static_cast(ErrorResultCode::CheckFailed);
   }
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Chec

[PATCH] D103538: [clangd] Run code completion on each token coverd by --check-lines

[Adam Czachorowski via Phabricator via cfe-commits]

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

I didn't put much thought into where --check-lines goes. It's an interesting 
thought.

I think having all the flags in one place is more valuable than trying to split 
them in some way. We contain all flags in the same place, close to main() and 
check() gets all it's data from arguments, rather than some side channel. It 
makes check() easier to re-use and/or test.  It also allows us to complain 
about flag misuse, such as specifying --check-lines without --check, etc.

I'm going to commit as-is, since you LGTMed this, but feel free to continue 
discussion here and if we decide to move the flags somewhere else I'll happily 
submit another change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103538

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

[clang] f917c5b - Revert test fixups after e9a9c850989e (which reverted a14fc74).

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2021-06-04T10:42:25-04:00
New Revision: f917c5b8d40b7894d52d56052bb18f8e989bad9e

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

LOG: Revert test fixups after e9a9c850989e (which reverted a14fc74).

This reverts commit da3ed58b97c1cc1356b7732d5dcbb6e4de3057da.
This reverts commit ba1fb0ff8c9f9ef7f9b7d1fe43cb95c8d1363f78.

Added: 


Modified: 
clang/test/Profile/c-linkage-available_externally.c
clang/test/Profile/c-linkage.c
clang/test/Profile/cxx-linkage.cpp

Removed: 




diff  --git a/clang/test/Profile/c-linkage-available_externally.c 
b/clang/test/Profile/c-linkage-available_externally.c
index c8dd9fc40a18a..5ac777b267ab8 100644
--- a/clang/test/Profile/c-linkage-available_externally.c
+++ b/clang/test/Profile/c-linkage-available_externally.c
@@ -3,7 +3,7 @@
 // RUN: %clang_cc1 -O2 -triple x86_64-apple-macosx10.9 -main-file-name 
c-linkage-available_externally.c %s -o - -emit-llvm -fprofile-instrument=clang 
| FileCheck %s
 
 // CHECK: @__profc_foo = linkonce_odr hidden global [1 x i64] zeroinitializer, 
section "__DATA,__llvm_prf_cnts", align 8
-// CHECK: @__profd_foo = private global {{.*}} i64* getelementptr inbounds ([1 
x i64], [1 x i64]* @__profc_foo, i32 0, i32 0){{.*}}, section 
"__DATA,__llvm_prf_data,regular,live_support", align 8
+// CHECK: @__profd_foo = linkonce_odr hidden global {{.*}} i64* getelementptr 
inbounds ([1 x i64], [1 x i64]* @__profc_foo, i32 0, i32 0){{.*}}, section 
"__DATA,__llvm_prf_data,regular,live_support", align 8
 inline int foo(void) { return 1; }
 
 int main(void) {

diff  --git a/clang/test/Profile/c-linkage.c b/clang/test/Profile/c-linkage.c
index 7a607e7237215..50ac558fb0761 100644
--- a/clang/test/Profile/c-linkage.c
+++ b/clang/test/Profile/c-linkage.c
@@ -4,7 +4,7 @@
 // CHECK: @__profc_foo = private global
 // CHECK: @__profd_foo = private global
 // CHECK: @__profc_foo_weak = weak hidden global
-// CHECK: @__profd_foo_weak = private global
+// CHECK: @__profd_foo_weak = weak hidden global
 // CHECK: @__profc_main = private global
 // CHECK: @__profd_main = private global
 // CHECK: @__profc_c_linkage.c_foo_internal = private global

diff  --git a/clang/test/Profile/cxx-linkage.cpp 
b/clang/test/Profile/cxx-linkage.cpp
index 99537c0e027ea..6f7b2b7128e9f 100644
--- a/clang/test/Profile/cxx-linkage.cpp
+++ b/clang/test/Profile/cxx-linkage.cpp
@@ -3,11 +3,11 @@
 // CHECK: @__profc__Z3foov = private global
 // CHECK: @__profd__Z3foov = private global
 // CHECK: @__profc__Z8foo_weakv = weak hidden global
-// CHECK: @__profd__Z8foo_weakv = private global
+// CHECK: @__profd__Z8foo_weakv = weak hidden global
 // CHECK: @__profc_main = private global
 // CHECK: @__profd_main = private global
 // CHECK: @__profc__Z10foo_inlinev = linkonce_odr hidden global
-// CHECK: @__profd__Z10foo_inlinev = private global
+// CHECK: @__profd__Z10foo_inlinev = linkonce_odr hidden global
 
 void foo(void) { }
 



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

[PATCH] D103646: [OPENMP]Fix PR50129: omp cancel parallel not working as expected.

[Johannes Doerfert via Phabricator via cfe-commits]

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

LG, one nit below.




Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:538
+omp::Directive CanceledDirective,
+FinalizeCallbackTy ExitCB = {});
 

The documentation doesn't match the prototype.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103646

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

[PATCH] D103642: [OPENMP]Fix PR49790: Constexpr values not handled in `omp declare mapper` clause.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG827b5c21545a: [OPENMP]Fix PR49790: Constexpr values not 
handled in `omp declare mapper`… (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103642

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_mapper_ast_print.cpp


Index: clang/test/OpenMP/declare_mapper_ast_print.cpp
===
--- clang/test/OpenMP/declare_mapper_ast_print.cpp
+++ clang/test/OpenMP/declare_mapper_ast_print.cpp
@@ -56,8 +56,9 @@
 // CHECK: #pragma omp declare mapper (id : dat::datin v) map(tofrom: 
v.in){{$}}
 // CHECK: };
 
-#pragma omp declare mapper(default : N1::vec kk) map(kk.len) map(kk.data[0:2])
-// CHECK: #pragma omp declare mapper (default : N1::vec kk) map(tofrom: 
kk.len) map(tofrom: kk.data[0:2]){{$}}
+constexpr int N = 2;
+#pragma omp declare mapper(default : N1::vec kk) map(kk.len) map(kk.data[0:N])
+// CHECK: #pragma omp declare mapper (default : N1::vec kk) map(tofrom: 
kk.len) map(tofrom: kk.data[0:N]){{$}}
 #pragma omp declare mapper(dat d) map(to: d.d)
 // CHECK: #pragma omp declare mapper (default : dat d) map(to: 
d.d){{$}}
 
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -19681,8 +19681,13 @@
 bool Sema::isOpenMPDeclareMapperVarDeclAllowed(const VarDecl *VD) const {
   assert(LangOpts.OpenMP && "Expected OpenMP mode.");
   const Expr *Ref = DSAStack->getDeclareMapperVarRef();
-  if (const auto *DRE = cast_or_null(Ref))
-return VD->getCanonicalDecl() == DRE->getDecl()->getCanonicalDecl();
+  if (const auto *DRE = cast_or_null(Ref)) {
+if (VD->getCanonicalDecl() == DRE->getDecl()->getCanonicalDecl())
+  return true;
+if (VD->isUsableInConstantExpressions(Context))
+  return true;
+return false;
+  }
   return true;
 }
 


Index: clang/test/OpenMP/declare_mapper_ast_print.cpp
===
--- clang/test/OpenMP/declare_mapper_ast_print.cpp
+++ clang/test/OpenMP/declare_mapper_ast_print.cpp
@@ -56,8 +56,9 @@
 // CHECK: #pragma omp declare mapper (id : dat::datin v) map(tofrom: v.in){{$}}
 // CHECK: };
 
-#pragma omp declare mapper(default : N1::vec kk) map(kk.len) map(kk.data[0:2])
-// CHECK: #pragma omp declare mapper (default : N1::vec kk) map(tofrom: kk.len) map(tofrom: kk.data[0:2]){{$}}
+constexpr int N = 2;
+#pragma omp declare mapper(default : N1::vec kk) map(kk.len) map(kk.data[0:N])
+// CHECK: #pragma omp declare mapper (default : N1::vec kk) map(tofrom: kk.len) map(tofrom: kk.data[0:N]){{$}}
 #pragma omp declare mapper(dat d) map(to: d.d)
 // CHECK: #pragma omp declare mapper (default : dat d) map(to: d.d){{$}}
 
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -19681,8 +19681,13 @@
 bool Sema::isOpenMPDeclareMapperVarDeclAllowed(const VarDecl *VD) const {
   assert(LangOpts.OpenMP && "Expected OpenMP mode.");
   const Expr *Ref = DSAStack->getDeclareMapperVarRef();
-  if (const auto *DRE = cast_or_null(Ref))
-return VD->getCanonicalDecl() == DRE->getDecl()->getCanonicalDecl();
+  if (const auto *DRE = cast_or_null(Ref)) {
+if (VD->getCanonicalDecl() == DRE->getDecl()->getCanonicalDecl())
+  return true;
+if (VD->isUsableInConstantExpressions(Context))
+  return true;
+return false;
+  }
   return true;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 827b5c2 - [OPENMP]Fix PR49790: Constexpr values not handled in `omp declare mapper` clause.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2021-06-04T07:32:14-07:00
New Revision: 827b5c21545aaa820403e9b5cced8c0181349ee2

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

LOG: [OPENMP]Fix PR49790: Constexpr values not handled in `omp declare mapper` 
clause.

Patch allows using of constexpr vars evaluatable to constant calue to be
used in declare mapper construct.

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

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/declare_mapper_ast_print.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 2fcc3fb4317e7..514b3b9ed05fe 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -19681,8 +19681,13 @@ Sema::ActOnOpenMPDeclareMapperDirectiveVarDecl(Scope 
*S, QualType MapperType,
 bool Sema::isOpenMPDeclareMapperVarDeclAllowed(const VarDecl *VD) const {
   assert(LangOpts.OpenMP && "Expected OpenMP mode.");
   const Expr *Ref = DSAStack->getDeclareMapperVarRef();
-  if (const auto *DRE = cast_or_null(Ref))
-return VD->getCanonicalDecl() == DRE->getDecl()->getCanonicalDecl();
+  if (const auto *DRE = cast_or_null(Ref)) {
+if (VD->getCanonicalDecl() == DRE->getDecl()->getCanonicalDecl())
+  return true;
+if (VD->isUsableInConstantExpressions(Context))
+  return true;
+return false;
+  }
   return true;
 }
 

diff  --git a/clang/test/OpenMP/declare_mapper_ast_print.cpp 
b/clang/test/OpenMP/declare_mapper_ast_print.cpp
index 6462fa38d872d..b9445ca35a0ca 100644
--- a/clang/test/OpenMP/declare_mapper_ast_print.cpp
+++ b/clang/test/OpenMP/declare_mapper_ast_print.cpp
@@ -56,8 +56,9 @@ class dat {
 // CHECK: #pragma omp declare mapper (id : dat::datin v) map(tofrom: 
v.in){{$}}
 // CHECK: };
 
-#pragma omp declare mapper(default : N1::vec kk) map(kk.len) map(kk.data[0:2])
-// CHECK: #pragma omp declare mapper (default : N1::vec kk) map(tofrom: 
kk.len) map(tofrom: kk.data[0:2]){{$}}
+constexpr int N = 2;
+#pragma omp declare mapper(default : N1::vec kk) map(kk.len) map(kk.data[0:N])
+// CHECK: #pragma omp declare mapper (default : N1::vec kk) map(tofrom: 
kk.len) map(tofrom: kk.data[0:N]){{$}}
 #pragma omp declare mapper(dat d) map(to: d.d)
 // CHECK: #pragma omp declare mapper (default : dat d) map(to: 
d.d){{$}}
 



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

[PATCH] D103612: [flang][driver] Add `-fno-unparse-typed-exprs`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 349868.
awarzynski added a comment.

Rebase on top of main, revert one small change uploaded accidentally


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103612

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/FrontendOptions.cpp
  flang/test/Driver/unparse-typed-exprs.f95
  flang/tools/f18/f18.cpp
  flang/tools/f18/flang

Index: flang/tools/f18/flang
===
--- flang/tools/f18/flang
+++ flang/tools/f18/flang
@@ -8,7 +8,7 @@
 #======#
 
 wd=$(cd $(dirname "$0")/.. && pwd)
-opts="-module-suffix .f18.mod "
+opts="-fno-unparse-typed-exprs -module-suffix .f18.mod "
 if ! $wd/bin/f18 $opts "$@"
 then status=$?
  echo flang: in $PWD, f18 failed with exit status $status: $wd/bin/f18 $opts "$@" >&2
Index: flang/tools/f18/f18.cpp
===
--- flang/tools/f18/f18.cpp
+++ flang/tools/f18/f18.cpp
@@ -105,7 +105,7 @@
   bool debugModuleWriter{false};
   bool defaultReal8{false};
   bool measureTree{false};
-  bool unparseTypedExprsToF18_FC{false};
+  bool unparseTypedExprs{true};
   std::vector F18_FCArgs;
   const char *prefix{nullptr};
   bool getDefinition{false};
@@ -322,7 +322,8 @@
 Unparse(llvm::outs(), parseTree, driver.encoding, true /*capitalize*/,
 options.features.IsEnabled(
 Fortran::common::LanguageFeature::BackslashEscapes),
-nullptr /* action before each statement */, &asFortran);
+nullptr /* action before each statement */,
+driver.unparseTypedExprs ? &asFortran : nullptr);
 return {};
   }
   if (driver.dumpPreFirTree) {
@@ -353,7 +354,7 @@
 options.features.IsEnabled(
 Fortran::common::LanguageFeature::BackslashEscapes),
 nullptr /* action before each statement */,
-driver.unparseTypedExprsToF18_FC ? &asFortran : nullptr);
+driver.unparseTypedExprs ? &asFortran : nullptr);
   }
 
   RunOtherCompiler(driver, tmpSourcePath.data(), relo.data());
@@ -578,8 +579,8 @@
 } else if (arg == "-funparse-with-symbols" ||
 arg == "-fdebug-unparse-with-symbols") {
   driver.dumpUnparseWithSymbols = true;
-} else if (arg == "-funparse-typed-exprs-to-f18-fc") {
-  driver.unparseTypedExprsToF18_FC = true;
+} else if (arg == "-fno-unparse-typed-exprs") {
+  driver.unparseTypedExprs = false;
 } else if (arg == "-fparse-only" || arg == "-fsyntax-only") {
   driver.syntaxOnly = true;
 } else if (arg == "-c") {
Index: flang/test/Driver/unparse-typed-exprs.f95
===
--- /dev/null
+++ flang/test/Driver/unparse-typed-exprs.f95
@@ -0,0 +1,34 @@
+! Tests `-fno-unparse-typed-exprs-to-f18-fc` frontend option
+
+!--
+! RUN lines
+!--
+! RUN: %flang_fc1 -fdebug-unparse  %s | FileCheck %s --check-prefix=UNPARSED_TYPED_EXPR
+! RUN: %flang_fc1 -fdebug-unparse -fno-unparse-typed-exprs %s | FileCheck %s --check-prefix=AS_FORTRAN
+
+!--
+! EXPECTED OUTPUT: default
+!--
+! UNPARSED_TYPED_EXPR: PROGRAM test_allocated
+! UNPARSED_TYPED_EXPR-NEXT:  INTEGER :: i = 4_4
+! UNPARSED_TYPED_EXPR-NEXT:  REAL(KIND=4_4), ALLOCATABLE :: x(:)
+! UNPARSED_TYPED_EXPR-NEXT:  IF (.NOT.allocated(x)) ALLOCATE(x(i))
+! UNPARSED_TYPED_EXPR-NEXT: END PROGRAM test_allocated
+
+!-
+! EXPECTED OUTPUT: unparsed as Fortran
+!--
+! AS_FORTRAN: PROGRAM test_allocated
+! AS_FORTRAN-NEXT:  INTEGER :: i = 4
+! AS_FORTRAN-NEXT:  REAL(KIND=4), ALLOCATABLE :: x(:)
+! AS_FORTRAN-NEXT:  IF (.NOT.allocated(x)) ALLOCATE(x(i))
+! AS_FORTRAN-NEXT: END PROGRAM test_allocated
+
+!--
+! INPUT
+!--
+program test_allocated
+  integer :: i = 4
+  real(4), allocatable :: x(:)
+  if (.not. allocated(x)) allocate(x(i))
+end program test_allocated
Index: flang/lib/Frontend/FrontendOptions.cpp
===
--- flang/lib/Frontend/FrontendOptions.cpp
+++ flang/lib/Frontend/FrontendOptions.cpp
@@ -32,34 +32,6 @@
   suffix == "F03" || suffix == "F08" || suffix == "F18";
 }
 
-// TODO: This is a copy of `asFortran` from f18.cpp and is added here for
-// compatiblity. It doesn't really belong here, but I couldn't find a better
-// place. We should decide whether to add it to the Evaluate or Parse/Unparse
-// APIs or some dedicated utility library in the driver.
-Fortran::parser::AnalyzedObjectsAsFortran
-Fortran::frontend::getBasicAsFortran() {
-  return Fort

[PATCH] D103587: [AIX] Transfer predefined macros

[Chris Bowler via Phabricator via cfe-commits]

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

I think it makes sense to split the __STDC macro changes and __TOS_AIX__ into 
different patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103587

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

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/UsersManual.rst:1484
+   Where unsafe floating point optimizations are enabled, this option
+   determines whether the optimizer honors parentheses when floating-point
+   expressions are evaluated.  If unsafe floating point optimizations are

mibintc wrote:
> aaron.ballman wrote:
> > We may need to expound on what "honor parentheses" means. The summary for 
> > the patch mentions `(a + b) + c`, but does this also mean `(x + y) * z`  
> > could be interpreted as `x + (y * z)`? What about for non-arithmetic 
> > expressions involving parentheses, like `(float1 == float2 && float1 == 
> > float3) || blah()`?
> Thanks for your review. Just a quick first response, What do you mean "(x + 
> y) * z could be reinterpreted as x + (y * z)" -- not following you here? 
"whether the optimizer honors the parentheses" reads to me like `(x + y) * z` 
could either honor or not honor the parentheses, so that could either evaluate 
as `(x + y) * z` or `x + (y * z)` depending on whether the parens are honored 
or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100118

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

[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

[Whisperity via Phabricator via cfe-commits]

whisperity marked 5 inline comments as done.
whisperity added a comment.

In D75041#2799023 , @martong wrote:

> Perhaps all conversion related logic should go into their own implementation 
> file? Seems like it adds up to roughly 1000 lines.

That's against the usual conventions of the project (1 TU for 1 check 
implementation). The methods are grouped by namespace, you can fold it up in 
your editor.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:550-551
+  if (isUselessSugar(LType.getTypePtr())) {
+LLVM_DEBUG(llvm::dbgs()
+   << "--- calculateMixability. LHS is useless sugar.\n");
 return calculateMixability(Check, LType.getSingleStepDesugaredType(Ctx),

martong wrote:
> Is this still WIP or do you use the DEBUG printouts in the tests? If not then 
> could you please create a Diff without the DEBUG printouts, that could 
> increase readability and ease the review.
As discussed earlier (and perhaps in the previous patches), these debug 
printouts are needed and shall stay here. The output is not used //in 
automation//, but it's intended for people who might pick up on further 
developing the check later. The recursive descent is too complex to be 
handleable in your mind without the debug information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75041

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

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

question for @aaron.ballman




Comment at: clang/docs/UsersManual.rst:1484
+   Where unsafe floating point optimizations are enabled, this option
+   determines whether the optimizer honors parentheses when floating-point
+   expressions are evaluated.  If unsafe floating point optimizations are

aaron.ballman wrote:
> We may need to expound on what "honor parentheses" means. The summary for the 
> patch mentions `(a + b) + c`, but does this also mean `(x + y) * z`  could be 
> interpreted as `x + (y * z)`? What about for non-arithmetic expressions 
> involving parentheses, like `(float1 == float2 && float1 == float3) || 
> blah()`?
Thanks for your review. Just a quick first response, What do you mean "(x + y) 
* z could be reinterpreted as x + (y * z)" -- not following you here? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100118

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

[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:550-551
+  if (isUselessSugar(LType.getTypePtr())) {
+LLVM_DEBUG(llvm::dbgs()
+   << "--- calculateMixability. LHS is useless sugar.\n");
 return calculateMixability(Check, LType.getSingleStepDesugaredType(Ctx),

Is this still WIP or do you use the DEBUG printouts in the tests? If not then 
could you please create a Diff without the DEBUG printouts, that could increase 
readability and ease the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75041

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

[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Perhaps all conversion related logic should go into their own implementation 
file? Seems like it adds up to roughly 1000 lines.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:1171
+   "conversion operator.\n");
+ImplicitSeq <<= ConversionOperatorResult.getValue();
+WorkType = ImplicitSeq.getTypeAfterUserDefinedConversion();

I think that these overloaded operators are not elevating the readability, 
since we are not dealing with mathematical classes.
IMHO, we could simply use `update` or something that is less crypto.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75041

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

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: rsmith, rjmccall.
aaron.ballman added inline comments.



Comment at: clang/docs/UsersManual.rst:1484
+   Where unsafe floating point optimizations are enabled, this option
+   determines whether the optimizer honors parentheses when floating-point
+   expressions are evaluated.  If unsafe floating point optimizations are

We may need to expound on what "honor parentheses" means. The summary for the 
patch mentions `(a + b) + c`, but does this also mean `(x + y) * z`  could be 
interpreted as `x + (y * z)`? What about for non-arithmetic expressions 
involving parentheses, like `(float1 == float2 && float1 == float3) || blah()`?



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8480-8481
+def err_typecheck_expect_flt_or_vector : Error<
+  "operand of type %0 where floating, complex or "
+  "a vector of such types is required (%0 invalid)">;
 def err_cast_selector_expr : Error<





Comment at: clang/include/clang/Basic/TargetInfo.h:1427
 
+  /// Controls if __arithmetic_fence is supported in the targetted backend.
+  virtual bool checkArithmeticFenceSupported() const { return false; }





Comment at: clang/include/clang/Basic/TargetInfo.h:1162
   /// the language based on the target options where applicable.
-  virtual void adjust(LangOptions &Opts);
+  virtual void adjust(DiagnosticsEngine &Diags, LangOptions &Opts);
 

mibintc wrote:
> There's no good place to diagnose when LangOptions and TargetOptions 
> conflict, I see "conflict" diagnostics being emitted in clang/CodeGen when 
> creating the func or module, which seems wrong to me. On the other hand, the 
> "adjust" function makes a good place to do the reconciliation I think. This 
> is the change that could be pulled out in a refactoring and committed prior 
> to this patch. What do you think? 
I think this makes sense, but should be done as a separate patch.



Comment at: clang/include/clang/Driver/Options.td:1757
+  LangOpts<"ProtectParens">, DefaultFalse,
+  PosFlaggetArg(0);
+  if (Arg->isInstantiationDependent())
+return false;

What if there is no argument given?



Comment at: clang/lib/Sema/SemaChecking.cpp:6425
+
+  const QualType ArgTy = TheCall->getArg(0)->getType();
+  bool IsFloating = [&]() {





Comment at: clang/lib/Sema/SemaChecking.cpp:6427-6430
+if (const VectorType *VT = dyn_cast(ArgTy.getCanonicalType()))
+  return VT->getElementType().getTypePtr()->isFloatingType();
+if (const ComplexType *CT = 
dyn_cast(ArgTy.getCanonicalType()))
+  return CT->getElementType().getTypePtr()->isFloatingType();





Comment at: clang/lib/Sema/SemaChecking.cpp:6436-6437
+   << ArgTy;
+  if (checkArgCount(*this, TheCall, 1))
+return true;
+  TheCall->setType(ArgTy);

This looks like it should move up a bit.



Comment at: clang/lib/Sema/SemaCoroutine.cpp:387
   JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
-  return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
-  JustAddress);
+  return S.BuildBuiltinCallExpr(Loc, Builtin::BI__builtin_coro_resume,
+JustAddress).get();

I am fine ignoring this clang-format issue; I think your formatting is far more 
readable.



Comment at: clang/lib/Sema/SemaExpr.cpp:4026
+  !E->isLValue() &&
+  (ExprTy->isFloatingType() || (ExprTy->isComplexType( {
+return BuildBuiltinCallExpr(R, Builtin::BI__arithmetic_fence, E);

Do you have to worry about vector types here as well?



Comment at: clang/lib/Sema/SemaExpr.cpp:4027
+  (ExprTy->isFloatingType() || (ExprTy->isComplexType( {
+return BuildBuiltinCallExpr(R, Builtin::BI__arithmetic_fence, E);
+  }

One worry I have about this is that the AST will be a bit mysterious for people 
writing AST matchers. The source code will have parens in it, but the AST will 
have a `ParenExpr` node or not only depending on the language options. This may 
also cause problems for other parts of the code that are checking for 
properly-parenthesized expressions (like && and || within the same expression), 
so we should probably have a test case like:
```
bool func(float f1, float f2, float f3) {
  return (f1 == f2 && f1 == f3) || f2 == f3; // Should not warn here
}
```



Comment at: clang/lib/Sema/SemaExpr.cpp:6535
+//  with the specified CallArgs
+ExprResult Sema::BuildBuiltinCallExpr(SourceLocation Loc, Builtin::ID Id,
+  MultiExprArg CallArgs) {

Because this cannot fail, is there a benefit to returning an `ExprResult` 
rather than an `Expr *` as before? Then you can remove a bunch of `ge

[PATCH] D103685: [clangd] Drop TestTUs dependency on gtest

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/unittests/TestTU.cpp:75
+  if (llvm::sys::fs::createUniqueDirectory("module-cache", ModuleCachePath))
+llvm_unreachable("Failed to create temp directory for module-cache");
   CI.getHeaderSearchOpts().ModuleCachePath = ModuleCachePath.c_str();

I don't love using llvm_unreachable for error handling (esp environment 
sensitive kind like this), it compiles down to UB in ndebug mode.

Log + abort would be preferable I think, though it's one extra line.



Comment at: clang-tools-extra/clangd/unittests/TestTU.cpp:119
+llvm::errs() << "Failed to build code:\n" << Code;
 llvm_unreachable("Failed to build TestTU!");
   }

I think we should replace this one with abort() too now



Comment at: clang-tools-extra/clangd/unittests/TestTU.cpp:122
   if (!AST->getDiagnostics()) {
-ADD_FAILURE() << "TestTU should always build an AST with a fresh Preamble"
-  << Code;
+llvm::errs() << "TestTU should always build an AST with a fresh Preamble"
+ << Code;

this check is now a log message that could still let the test pass
Make it an assert instead?

(I think assert without any early return is appropriate per normal llvm style)



Comment at: clang-tools-extra/clangd/unittests/TestTU.cpp:148
 << Code;
 break; // Just report first error for simplicity.
   }

this needs to fail the test. abort()?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103685

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

[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:122
 
 #define FB(Name, K) MIX_##Name = (1ull << (K##ull - 1ull))
 

whisperity wrote:
> martong wrote:
> > FB stands for FunnyBitmask? Could you please either describe that in a 
> > comment or just spell it out?
> FlagBit. @alexfh suggested in the base check to use hexa literals. I'm not 
> too sold about that, but maybe we can cut the macro out and keep the bit 
> shift instructions in. Given that the check has more or less earned its final 
> form (for now), we know how many bits we need!
FlagBit, I like it, perhaps we should rename `FB` to `FlagBit`?



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:182
+  /// The intermediate type after the first Standard Conversion Sequence.
+  QualType AfterPreStd;
+

whisperity wrote:
> martong wrote:
> > Maybe it's just me, but AfterPre sounds ambiguous and AfterPost seems 
> > redundant.
> Should I use "AfterFirstStandard" or "AfterFirstStd"?
> 
> AfterPost isn't redundant. An implicit conversion sequence might be of **3 
> steps**. Consider in a hypothetical `void f(Complex x, const double d)`.
> 
> `Complex` ---(PRE: std qual adjustment)--> `const Complex` ---(UD: user 
> defined conv)--> `double` ---(POST: std qual adjustment)--> `const double`
> 
> And because in case of many conversions a lot of combinations need to be 
> checked, we can't have `AfterPost` be the same as `End`. First, because of 
> the combinations, second, because of the //other// things the check is doing.
> 
> `void g(ComplexTypedef CT, ConstDoubleTypedef CDT);`
> 
> In this case, `Start` and `End` are the typedefs, and the inner sequence is 
> the same as before. And in order to generate the diagnostic, we also need to 
> have **both** pieces of information.
Thanks for this explanation, it makes clearer! However, this highlights that a 
way better description (in the form of comments) is needed for these `QualType` 
members. Especially this line could really increase the readability.
```
Complex ---(PRE: std qual adjustment)--> const Complex ---(UD: user defined 
conv)--> double ---(POST: std qual adjustment)--> const double
```

> In this case, Start and End are the typedefs, and the inner sequence is the 
> same as before. And in order to generate the diagnostic, we also need to have 
> both pieces of information.

I think it would be useful to document the cases with examples when `End` is 
not equal to `AfterPost` and such.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:210
+  /// the conversion sequence. This method does **NOT** return Begin and End.
+  SmallVector getInvolvedTypesInSequence() const {
+SmallVector Ret;

whisperity wrote:
> martong wrote:
> > So, we can never return with a vector with size > 4?
> `N` in `SmallVector` only specifies the pre-allocated smallness size. 
> AFAIK, if you have >N elements, it will just put the vector out onto the heap.
I meant the number 4, not SmallVector. So, is there a case when the conversion 
sequence is longer than 4? If it can be longer, then where do you store the 
types, you have 4 `QualType` members if I am not wrong. (Also, I am not an 
expert of conversions.) 

To be honest, it is so terrible that we cannot reuse [[ 
https://clang.llvm.org/doxygen/classclang_1_1StandardConversionSequence.html | 
clang::StandardConversionSequence ]]



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h:42
 
-  /// Whether to consider an unqualified and a qualified type mixable.
+  /// Whether to consider differently qualified versions of the same type
+  /// mixable.

whisperity wrote:
> martong wrote:
> > "qualified"
> > Do you consider `volatile` here as well, or just `const`?
> Const, volatile, and in C mode, restrict, too.
Great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75041

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

[clang] 86c2449 - [OpenCL][NFC] Test commit: tidy up whitespace in comment

[Stuart Brady via cfe-commits]

Author: Stuart Brady
Date: 2021-06-04T14:44:12+01:00
New Revision: 86c24493ea666a0ef91b7af884d616b0a181e849

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

LOG: [OpenCL][NFC] Test commit: tidy up whitespace in comment

Added: 


Modified: 
clang/lib/Headers/opencl-c.h

Removed: 




diff  --git a/clang/lib/Headers/opencl-c.h b/clang/lib/Headers/opencl-c.h
index dcd325528f38b..9531e534a61bc 100644
--- a/clang/lib/Headers/opencl-c.h
+++ b/clang/lib/Headers/opencl-c.h
@@ -6400,8 +6400,7 @@ size_t __ovld __cnfn get_local_id(uint dimindx);
  * Returns the number of work-groups that will execute a
  * kernel for dimension identified by dimindx.
  * Valid values of dimindx are 0 to get_work_dim() - 1.
- * For other values of dimindx, get_num_groups () returns
- * 1.
+ * For other values of dimindx, get_num_groups() returns 1.
  * For clEnqueueTask, this always returns 1.
  */
 size_t __ovld __cnfn get_num_groups(uint dimindx);



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

[PATCH] D103587: [AIX] Transfer predefined macros

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan updated this revision to Diff 349855.
Jake-Egan added a comment.

Use LangStandard::isC11() and one C99 test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103587

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/test/Preprocessor/init-ppc.c


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -541,6 +541,7 @@
 // PPC-AIX:#define __SIZE_MAX__ 4294967295UL
 // PPC-AIX:#define __SIZE_TYPE__ long unsigned int
 // PPC-AIX:#define __SIZE_WIDTH__ 32
+// PPC-AIX:#define __TOS_AIX__ 1
 // PPC-AIX:#define __UINT16_C_SUFFIX__
 // PPC-AIX:#define __UINT16_MAX__ 65535
 // PPC-AIX:#define __UINT16_TYPE__ unsigned short
@@ -723,6 +724,25 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
PPC-AIX-NOTHREADSAFE %s
 // PPC-AIX-NOTHREADSAFE-NOT:#define _THREAD_SAFE 1
 
+// RUN: %clang_cc1 -x c -std=c11 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=c1x -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=iso9899:2011 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.10.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=gnu11 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=c17 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=iso9899:2017 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=c18 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=iso9899:2018 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=gnu17 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=gnu18 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=c2x -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=gnu2x -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char  < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC %s
+// PPC-AIX-STDC:#define __STDC_NO_ATOMICS__ 1
+// PPC-AIX-STDC:#define __STDC_NO_THREADS__ 1
+
+// RUN: %clang_cc1 -x c -std=c99 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC-N %s
+// PPC-AIX-STDC-N-NOT:#define __STDC_NO_ATOMICS__ 1
+// PPC-AIX-STDC-N-NOT:#define __STDC_NO_THREADS__ 1
+
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-unknown-linux-gnu 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
PPC-LINUX %s
 //
 // PPC-LINUX:#define _ARCH_PPC 1
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -675,6 +675,12 @@
 Builder.defineMacro("_POWER");
 
 Builder.defineMacro("_AIX");
+Builder.defineMacro("__TOS_AIX__");
+
+if (LangStandard::getLangStandardForKind(Opts.LangStd).isC11()) {
+  Builder.defineMacro("__STDC_NO_ATOMICS__");
+  Builder.defineMacro("__STDC_NO_THREADS__");
+}
 
 if (Opts.EnableAIXExtendedAltivecABI)
   Builder.defineMacro("__EXTABI__");


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -541,6 +541,7 @@
 // PPC-AIX:#define __SIZE_MAX__ 4294967295UL
 // PPC-AIX:#define __SIZE_TYPE__ long unsigned int
 // PPC-AIX:#define __SIZE_WIDTH__ 32
+// PPC-AIX:#define __TOS_AIX__ 1
 // PPC-AIX:#define __UINT16_C_SUFFIX__
 // PPC-AIX:#define __UINT16_MAX__ 65535
 // PPC-AIX:#define __UINT16_TYPE__ unsigned short
@@ -723,6 +724,25 @@
 //

[PATCH] D101630: [HIP] Fix device-only compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D101630#2792160 , @tra wrote:

> In D101630#2792052 , @yaxunl wrote:
>
>> I think for intermediate outputs e.g. preprocessor expansion, IR, and 
>> assembly, probably it makes sense not to bundle by default.
>
> Agreed.
>
>> However, for default action (emitting object), we need to bundle by default 
>> since it was the old behavior and existing HIP apps depend on that.
>
> Existing use is a valid point.
> As a counterargument, I would suggest that in a compilation pipeline which 
> does include bundling, an object file for one GPU variant *is* an 
> intermediate output, similar to the ones you've listed above.
>
> The final product of device-side subcompilations is a bundle. The question is 
> `what does "-c" mean?`.  Is it `produce an object file` or `compile till the 
> end of the pipeline` ? 
> For CUDA and HIP compilation it's ambiguous. When we target just one GPU, it 
> would be closer to the former. In general, it would be closer to the latter. 
> NVCC side-steps the issue by using a different flags `-cubin/-fatbin` to 
> disambiguate between two cases and avoid bolting on CUDA-related semantics on 
> the compiler flags that were not designed for that.
>
>> Then we allow -fhip-bundle-device-output to override the default behavior.
>
> OK. Bundling objects for HIP by default looks like a reasonable compromise. 
> It would be useful to generalize the flag to `-fgpu-bundle...` as it would be 
> useful if/when we want to produce a fatbin during CUDA compilation. I'd still 
> keep no-bundling as the default for CUDA's objects.
>
> Now that we are in agreement of what we want, the next question is *how* we 
> want to do it.
>
> It appears that there's a fair bit of similarity between what the proposed 
> `-fgpu-bundle` flag does and the handful of `--emit-...` options clang has 
> now.
> If we were to use something like `--emit-gpu-object` and `--emit-gpu-bundle`, 
> it would be similar to NVCC's `-cubin/-fatbinary`, would decouple the default 
> behavior for `-c --cuda-device-only` from the user's ability to specify what 
> they want without burdening `-c` with additional flags that would have 
> different defaults under different circumstances.
>
> Compilation with "-c" would remain the "compile till the end", whatever it 
> happens to mean for particular language and `--emit-object/bundle` would tell 
> the compiler how far we want it to proceed and what kind of output we want. 
> This would probably be easier to explain to the users as they are already 
> familiar with flags like `-emit-llvm`, only now we are dealing with an extra 
> bundling step in the compilation pipeline. It would also behave consistently 
> across CUDA and HIP even though they have different defaults for bundling for 
> the device-side compilation. E.g. `-c --cuda-device-only --emit-gpu-bundle` 
> will always produce a bundle with the object files for both CUDA and HIP and 
> `-c --cuda-device-only --emit-gpu-object` will always require single '-o' 
> output.
>
> WDYT? Does it make sense?

For sure we will need -fgpu-bundle-device-output to control bundling of 
intermediate files. Then adding -emit-gpu-object and -emit-gpu-bundle may be 
redundant and can cause confusion. What if users specify `-c 
-fgpu-bundle-device-output -emit-gpu-object` or `-c 
-fno-gpu-bundle-device-output -emit-gpu-bundle`? To me a single option 
-fgpu-bundle-device-output to control all device output seems cleaner.


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

https://reviews.llvm.org/D101630

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

[PATCH] D103587: [AIX] Transfer predefined macros

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan updated this revision to Diff 349854.
Jake-Egan marked 3 inline comments as done.
Jake-Egan added a comment.

Use LangStandard::isC11() and one C99 test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103587

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/test/Preprocessor/init-ppc.c


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -541,6 +541,7 @@
 // PPC-AIX:#define __SIZE_MAX__ 4294967295UL
 // PPC-AIX:#define __SIZE_TYPE__ long unsigned int
 // PPC-AIX:#define __SIZE_WIDTH__ 32
+// PPC-AIX:#define __TOS_AIX__ 1
 // PPC-AIX:#define __UINT16_C_SUFFIX__
 // PPC-AIX:#define __UINT16_MAX__ 65535
 // PPC-AIX:#define __UINT16_TYPE__ unsigned short
@@ -723,6 +724,10 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
PPC-AIX-NOTHREADSAFE %s
 // PPC-AIX-NOTHREADSAFE-NOT:#define _THREAD_SAFE 1
 
+// RUN: %clang_cc1 -x c -std=c99 -E -dM -ffreestanding 
-triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix PPC-AIX-STDC-N %s
+// PPC-AIX-STDC-N-NOT:#define __STDC_NO_ATOMICS__ 1
+// PPC-AIX-STDC-N-NOT:#define __STDC_NO_THREADS__ 1
+
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-unknown-linux-gnu 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
PPC-LINUX %s
 //
 // PPC-LINUX:#define _ARCH_PPC 1
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -675,6 +675,12 @@
 Builder.defineMacro("_POWER");
 
 Builder.defineMacro("_AIX");
+Builder.defineMacro("__TOS_AIX__");
+
+if (LangStandard::getLangStandardForKind(Opts.LangStd).isC11()) {
+  Builder.defineMacro("__STDC_NO_ATOMICS__");
+  Builder.defineMacro("__STDC_NO_THREADS__");
+}
 
 if (Opts.EnableAIXExtendedAltivecABI)
   Builder.defineMacro("__EXTABI__");


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -541,6 +541,7 @@
 // PPC-AIX:#define __SIZE_MAX__ 4294967295UL
 // PPC-AIX:#define __SIZE_TYPE__ long unsigned int
 // PPC-AIX:#define __SIZE_WIDTH__ 32
+// PPC-AIX:#define __TOS_AIX__ 1
 // PPC-AIX:#define __UINT16_C_SUFFIX__
 // PPC-AIX:#define __UINT16_MAX__ 65535
 // PPC-AIX:#define __UINT16_TYPE__ unsigned short
@@ -723,6 +724,10 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-NOTHREADSAFE %s
 // PPC-AIX-NOTHREADSAFE-NOT:#define _THREAD_SAFE 1
 
+// RUN: %clang_cc1 -x c -std=c99 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC-N %s
+// PPC-AIX-STDC-N-NOT:#define __STDC_NO_ATOMICS__ 1
+// PPC-AIX-STDC-N-NOT:#define __STDC_NO_THREADS__ 1
+
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-unknown-linux-gnu -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-LINUX %s
 //
 // PPC-LINUX:#define _ARCH_PPC 1
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -675,6 +675,12 @@
 Builder.defineMacro("_POWER");
 
 Builder.defineMacro("_AIX");
+Builder.defineMacro("__TOS_AIX__");
+
+if (LangStandard::getLangStandardForKind(Opts.LangStd).isC11()) {
+  Builder.defineMacro("__STDC_NO_ATOMICS__");
+  Builder.defineMacro("__STDC_NO_THREADS__");
+}
 
 if (Opts.EnableAIXExtendedAltivecABI)
   Builder.defineMacro("__EXTABI__");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D103314#2790868 , @vsavchenko 
wrote:

> Awesome!
> I know, I said that we are ready to land, but I think I was too excited about 
> this change. We probably should have some data on how it performs on 
> real-life codebases.

Just some quick update on the status of this patch. I've done some measurements 
on smaller open source C projects (e.g tmux) and didn't see any noticeable 
slow-down. However, I've run into a bad-bad assertion failure in my favorite 
Checker (StdLibraryFu...). The assertion indicates that neither !State nor 
State is feasible, so this throws me back to the debugger for a while.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103314

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

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D103605#2798171 , @NoQ wrote:

> Ok. Oof. Whoa. That's amazing.
>
> Let me re-iterate to make sure that my understanding of this patchset is 
> correct.
>
> **Chapter 1.** //"A is correlated with B (ρ = 0.56), given C, assuming D and 
> under E conditions"//
>
> - Two new entities are introduced: trackers and handlers.
> - Trackers are attached to bug reports. A single bug report may have multiple 
> trackers.
>   - Why not have only one tracker per report?
> - Handlers are attached to trackers. Each tracker can have multiple handlers 
> with a priority system.
>   - Do you envision it growing into a dependency system later?
> - A subclass of visitors known as "tracking visitors" are connected to 
> trackers. One tracker may have multiple visitors attached to it.
> - Trackers are a tiny, almost uncustomizable class. It contains is a method 
> called `track()` that is invoked when attention of handlers is required.
> - Each tracker comes with a default set of handlers.
> - The only way to customize a tracker is to add extra handlers to it.
> - Handlers implement a `handle()` method invoked by the tracker every time 
> their attention is required in `track()`.
> - Handlers may return a note from their `handle()` method. If a handler 
> returns a note, this note is attached to the bug report and lower-priority 
> handlers are not invoked at all.
> - There are two major classes of handlers: expression handlers and store 
> handlers. Expression handlers pay attention to Environment entries, store 
> handlers pay attention to Region Store bindings.
>
> **Chapter 2.** //"For immediate release: Scientists find potential link 
> between A and B"//
>
> - Bug visitors are still responsible for discovering connections across 
> exploded nodes. On the contrary, `track()`/`handle()` workflow is 
> instantaneous.
>   - It sounds like this whole patchset is orthogonal to reimplementing value 
> tracking visitors with note tags, as long as we can pass a reference to the 
> appropriate tracker into the tag(?)
> - Tracking starts when a checker creates a tracker and invokes `track()` on 
> it, which instantly causes the respective `handle()` to be invoked.
> - `handle()` may attach visitors. These visitors may in turn call `track()` 
> as they reach a different exploded node.
> - The termination condition is as usual: no further visitors attached.
>   - It sounds like this patchset is orthogonal to the problem of detecting 
> the tracking termination point(?) It does make it easier to implement 
> customized reactions to such termination though.
>
> **Chapter 3.** //"A causes B all the time. What will this means for Obama?"//
>
> - `trackExpressionValue()` now boils down to creation of a single tracker and 
> an initial invocation of `track()` on the expression which in turn causes 
> expression handlers to immediately react.
> - Most of the old body of `trackExpressionValue()` is transformed into a 
> default expression handler. So it's still part of an immediate reaction, just 
> a bit more generalized.
> - Most of the visitors that previously composed `trackExpressionValue()` are 
> still part of the process; they're attached by the default expression handler.
> - However, instead of adding notes directly, they simply invoke `track()`. 
> All note-attaching work is moved into additional default handlers.
>
> **Chapter 4.** //"I'm wearing this to ward off A!"//
>
> - Basic usage in checkers is basically unchanged. They still simply call 
> `trackExpressionValue()` and it just works.
> - Advanced usage - which is the whole point of this patchset - probably 
> implies adding custom handlers to the tracker created by 
> `trackExpressionValue()`.
> - Let's consider an example. Suppose you're implementing `evalCall` for 
> `llvm::dyn_cast`. You could have your expression handler detect a modeled 
> `dyn_cast` call and implement its `handle()` method to emit a 
> checker-specific note and start tracking the argument.
> - With this implementation such note will only be emitted if the value is 
> tracked back to `dyn_cast`. Not every modeled `dyn_cast` will receive a note 
> but only the one that, say, caused a null dereference on its return value 
> when it failed.
>   - Now, how does this interact with note tags? The idea behind note tags is 
> so that visitors didn't need to reverse-engineer the analysis.
>   - It sounds like the handler is a step back to the visitor world: it 
> doesn't know how exactly was `dyn_cast` modeled during analysis, so it has to 
> reverse-engineer what happened.
>   - For example, in order to find out whether `dyn_cast` succeeded or failed, 
> it has to explore the program state to find out which values are passed into 
> the cast and returned from the cast.
>   - Whereas a note tag would have this information accessible directly.
> - Hence a suggestion. Maybe instead of having a system of handlers, allow 
> visitors to //ask

[PATCH] D103281: [HIP] Fix spack HIP device lib detection

[Yaxun Liu 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 rGb5dea8701ba9: [HIP] Fix spack HIP device lib detection 
(authored by yaxunl).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103281

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/asanrtl.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/hip.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/ockl.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_correctly_rounded_sqrt_off.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_correctly_rounded_sqrt_on.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_daz_opt_off.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_daz_opt_on.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_finite_only_off.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_finite_only_on.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_1010.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_1011.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_1012.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_803.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_900.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_908.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_unsafe_math_off.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_unsafe_math_on.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_wavefrontsize64_off.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_wavefrontsize64_on.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/ocml.bc
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/opencl.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/asanrtl.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/hip.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/ockl.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_correctly_rounded_sqrt_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_correctly_rounded_sqrt_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_daz_opt_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_daz_opt_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_finite_only_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_finite_only_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_1010.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_1011.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_1012.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_803.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_900.bc
  
clang/test/D

[clang] b5dea87 - [HIP] Fix spack HIP device lib detection

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-06-04T09:12:41-04:00
New Revision: b5dea8701ba98425991d4f1ec3d87bdb98789e04

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

LOG: [HIP] Fix spack HIP device lib detection

spack HIP device library is installed at amdgcn directory under llvm/clang
directory.

This patch fixes detection of HIP device library for spack.

Reviewed by: Artem Belevich, Harmen Stoppels

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

Added: 

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/asanrtl.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/hip.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/ockl.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_correctly_rounded_sqrt_off.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_correctly_rounded_sqrt_on.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_daz_opt_off.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_daz_opt_on.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_finite_only_off.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_finite_only_on.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_1010.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_1011.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_1012.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_803.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_900.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_908.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_unsafe_math_off.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_unsafe_math_on.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_wavefrontsize64_off.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_wavefrontsize64_on.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/ocml.bc

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/opencl.bc

Modified: 
clang/lib/Driver/ToolChains/AMDGPU.cpp
clang/test/Driver/rocm-detect.hip

Removed: 

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/asanrtl.bc

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/hip.bc

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/ockl.bc

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_correctly_rounded_sqrt_off.bc

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_correctly_rounded_sqrt_on.bc

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_daz_opt_off.bc

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_daz_opt_on.bc

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_finite_only_off.bc

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_finite_only_on.bc

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_1010.bc

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_1011.bc

clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_

[PATCH] D103519: [clang][deps] Support object files

[Jan Svoboda 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 rGde07b1e84d8d: [clang][deps] Support object files (authored 
by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D103519?vs=349245&id=349847#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103519

Files:
  clang/lib/Tooling/DependencyScanning/CMakeLists.txt
  clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json
  clang/test/ClangScanDeps/Inputs/modules-pch/mod_tu.h
  clang/test/ClangScanDeps/Inputs/modules-pch/module.modulemap
  clang/test/ClangScanDeps/Inputs/modules-pch/pch.h
  clang/test/ClangScanDeps/Inputs/modules-pch/tu.c
  clang/test/ClangScanDeps/modules-pch.c

Index: clang/test/ClangScanDeps/modules-pch.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: cp %S/Inputs/modules-pch/* %t
+
+// Explicitly build the PCH:
+//
+// RUN: %clang -x c-header %t/pch.h -fmodules -gmodules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t/cache -o %t/pch.h.gch
+
+// Scan dependencies of the TU:
+//
+// RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_tu.json > %t/cdb_tu.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_tu.json -format experimental-full \
+// RUN:   -generate-modules-path-args -module-files-dir %t/build
Index: clang/test/ClangScanDeps/Inputs/modules-pch/tu.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/modules-pch/tu.c
@@ -0,0 +1,3 @@
+// tu.c
+
+#include "mod_tu.h"
Index: clang/test/ClangScanDeps/Inputs/modules-pch/pch.h
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/modules-pch/pch.h
@@ -0,0 +1 @@
+// pch.h
Index: clang/test/ClangScanDeps/Inputs/modules-pch/module.modulemap
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/modules-pch/module.modulemap
@@ -0,0 +1,3 @@
+module ModTU {
+header "mod_tu.h"
+}
Index: clang/test/ClangScanDeps/Inputs/modules-pch/mod_tu.h
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/modules-pch/mod_tu.h
@@ -0,0 +1 @@
+// mod_tu.h
Index: clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json
@@ -0,0 +1,7 @@
+[
+  {
+"directory": "DIR",
+"command": "clang -fsyntax-only DIR/tu.c -fmodules -gmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -include DIR/pch.h -o DIR/tu.o",
+"file": "DIR/tu.c"
+  }
+]
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
+#include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -153,7 +154,15 @@
 DependencyScanningService &Service)
 : Format(Service.getFormat()) {
   DiagOpts = new DiagnosticOptions();
+
   PCHContainerOps = std::make_shared();
+  PCHContainerOps->registerReader(
+  std::make_unique());
+  // We don't need to write object files, but the current PCH implementation
+  // requires the writer to be registered as well.
+  PCHContainerOps->registerWriter(
+  std::make_unique());
+
   RealFS = llvm::vfs::createPhysicalFileSystem();
   if (Service.canSkipExcludedPPRanges())
 PPSkipMappings =
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "llvm/Support/TargetSelect.h"
 
 using namespace clang;
 using namespace tooling;
@@ -16,4 +17,10 @@
 ScanningMode Mode, ScanningOutputFormat Format, bool ReuseFileManager,
 bool SkipExcludedPPRanges)
 : Mode(Mode), Format(Format), ReuseFi

[PATCH] D103613: [flang][driver] Add support for `-module-suffix`

[Andrzej Warzynski via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG20bd2142d465: [flang][driver] Add support for 
`-module-suffix` (authored by awarzynski).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103613

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Driver/module-suffix.f90

Index: flang/test/Driver/module-suffix.f90
===
--- /dev/null
+++ flang/test/Driver/module-suffix.f90
@@ -0,0 +1,16 @@
+! Tests `-module-suffix` frontend option
+
+!--
+! RUN lines
+!--
+! RUN: rm -rf %t && mkdir -p %t/dir-flang/
+! RUN: cd %t && %flang_fc1 -fsyntax-only -module-suffix .f18.mod -module-dir %t/dir-flang %s
+! RUN: ls %t/dir-flang/testmodule.f18.mod && not ls %t/dir-flang/testmodule.mod
+
+!--
+! INPUT
+!--
+module testmodule
+  type::t2
+  end type
+end
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -106,6 +106,7 @@
 ! HELP-FC1-NEXT: -help  Display available options
 ! HELP-FC1-NEXT: -IAdd directory to the end of the list of include search paths
 ! HELP-FC1-NEXT: -module-dir   Put MODULE files in 
+! HELP-FC1-NEXT: -module-suffix  Use  as the suffix for module files (the default value is `.mod`)
 ! HELP-FC1-NEXT: -nocpp Disable predefined and command line preprocessor macros
 ! HELP-FC1-NEXT: -o   Write output to 
 ! HELP-FC1-NEXT: -pedantic  Warn on language extensions
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -392,6 +392,12 @@
 res.SetDebugModuleDir(true);
   }
 
+  // -module-suffix
+  if (const auto *moduleSuffix =
+  args.getLastArg(clang::driver::options::OPT_module_suffix)) {
+res.SetModuleFileSuffix(moduleSuffix->getValue());
+  }
+
   return diags.getNumErrors() == numErrorsBefore;
 }
 
@@ -639,5 +645,6 @@
   semanticsContext_->set_moduleDirectory(moduleDir())
   .set_searchDirectories(fortranOptions.searchDirectories)
   .set_warnOnNonstandardUsage(enableConformanceChecks())
-  .set_warningsAreErrors(warnAsErr());
+  .set_warningsAreErrors(warnAsErr())
+  .set_moduleFileSuffix(moduleFileSuffix());
 }
Index: flang/include/flang/Frontend/CompilerInvocation.h
===
--- flang/include/flang/Frontend/CompilerInvocation.h
+++ flang/include/flang/Frontend/CompilerInvocation.h
@@ -69,6 +69,8 @@
   // of options.
   std::string moduleDir_ = ".";
 
+  std::string moduleFileSuffix_ = ".mod";
+
   bool debugModuleDir_ = false;
 
   bool warnAsErr_ = false;
@@ -97,6 +99,9 @@
   std::string &moduleDir() { return moduleDir_; }
   const std::string &moduleDir() const { return moduleDir_; }
 
+  std::string &moduleFileSuffix() { return moduleFileSuffix_; }
+  const std::string &moduleFileSuffix() const { return moduleFileSuffix_; }
+
   bool &debugModuleDir() { return debugModuleDir_; }
   const bool &debugModuleDir() const { return debugModuleDir_; }
 
@@ -129,6 +134,10 @@
   /// Useful setters
   void SetModuleDir(std::string &moduleDir) { moduleDir_ = moduleDir; }
 
+  void SetModuleFileSuffix(const char *moduleFileSuffix) {
+moduleFileSuffix_ = std::string(moduleFileSuffix);
+  }
+
   void SetDebugModuleDir(bool flag) { debugModuleDir_ = flag; }
 
   void SetWarnAsErr(bool flag) { warnAsErr_ = flag; }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4522,6 +4522,9 @@
 def fget_symbols_sources : Flag<["-"], "fget-symbols-sources">, Group,
   HelpText<"Dump symbols and their source code locations">;
 
+def module_suffix : Separate<["-"], "module-suffix">,  Group, MetaVarName<"">,
+  HelpText<"Use  as the suffix for module files (the default value is `.mod`)">;
+
 }
 
 //===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] de07b1e - [clang][deps] Support object files

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-04T14:58:42+02:00
New Revision: de07b1e84d8de948304766df602fee2b845e9532

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

LOG: [clang][deps] Support object files

When a project uses PCH with explicit modules, the build will look like this:

1. scan PCH dependencies
2. explicitly build PCH
3. scan TU dependencies
4. explicitly build TU

Step 2 produces an object file for the PCH, which the dependency scanner needs 
to read in step 3. This patch adds support for this.

The `clang-scan-deps` invocation in the attached test would fail without this 
change.

Depends on D103516.

Reviewed By: Bigcheese

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

Added: 
clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json
clang/test/ClangScanDeps/Inputs/modules-pch/mod_tu.h
clang/test/ClangScanDeps/Inputs/modules-pch/module.modulemap
clang/test/ClangScanDeps/Inputs/modules-pch/pch.h
clang/test/ClangScanDeps/Inputs/modules-pch/tu.c
clang/test/ClangScanDeps/modules-pch.c

Modified: 
clang/lib/Tooling/DependencyScanning/CMakeLists.txt
clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/CMakeLists.txt 
b/clang/lib/Tooling/DependencyScanning/CMakeLists.txt
index c6fe207ab2f27..ce455e518070b 100644
--- a/clang/lib/Tooling/DependencyScanning/CMakeLists.txt
+++ b/clang/lib/Tooling/DependencyScanning/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
   Core
   Support
   )
@@ -16,6 +17,7 @@ add_clang_library(clangDependencyScanning
   LINK_LIBS
   clangAST
   clangBasic
+  clangCodeGen
   clangDriver
   clangFrontend
   clangFrontendTool

diff  --git 
a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
index 93bb0cde439dc..4f3e574719d2b 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "llvm/Support/TargetSelect.h"
 
 using namespace clang;
 using namespace tooling;
@@ -16,4 +17,10 @@ DependencyScanningService::DependencyScanningService(
 ScanningMode Mode, ScanningOutputFormat Format, bool ReuseFileManager,
 bool SkipExcludedPPRanges)
 : Mode(Mode), Format(Format), ReuseFileManager(ReuseFileManager),
-  SkipExcludedPPRanges(SkipExcludedPPRanges) {}
+  SkipExcludedPPRanges(SkipExcludedPPRanges) {
+  // Initialize targets for object file support.
+  llvm::InitializeAllTargets();
+  llvm::InitializeAllTargetMCs();
+  llvm::InitializeAllAsmPrinters();
+  llvm::InitializeAllAsmParsers();
+}

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 63264b0dda2d5..ecaeeed1a061c 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
+#include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -153,7 +154,15 @@ DependencyScanningWorker::DependencyScanningWorker(
 DependencyScanningService &Service)
 : Format(Service.getFormat()) {
   DiagOpts = new DiagnosticOptions();
+
   PCHContainerOps = std::make_shared();
+  PCHContainerOps->registerReader(
+  std::make_unique());
+  // We don't need to write object files, but the current PCH implementation
+  // requires the writer to be registered as well.
+  PCHContainerOps->registerWriter(
+  std::make_unique());
+
   RealFS = llvm::vfs::createPhysicalFileSystem();
   if (Service.canSkipExcludedPPRanges())
 PPSkipMappings =

diff  --git a/clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json 
b/clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json
new file mode 100644
index 0..8aad2cc74023e
--- /dev/null
+++ b/clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json
@@ -0,0 +1,7 @@
+[
+  {
+"directory": "DIR",
+"command": "clang -fsyntax-only DIR/tu.c -fmodules -gmodules 
-fimplicit-module-maps -fmodules-cache-path=DIR/cache -include DIR/pch.h -o 
DIR/tu.o",
+"file": "DIR/tu.c"
+  }
+