[PATCH] D103131: support debug info for alias variable

2021-06-09 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment.

In D103131#2791881 , @dblaikie wrote:

> In D103131#2789493 , @probinson 
> wrote:
>
>>> Mixed feelings - somewhat in favor of "do the thing that's probably already 
>>> fairly tested/known to work" (GCC's thing). But open to the idea that that 
>>> approach has problems, for sure.
>>
>> "Known to work" for whom?  gdb, sure, because gcc/gdb cooperate on many 
>> things.
>
> Yep. Given the diversity of ways of expressing things in DWARF, no consumer 
> will handle everything that could be produced in the way a producer might 
> intend - so we do sort of have a defacto standard of "what would GCC do/what 
> does GDB understand".
>
> But also GCC/GDB has had more implementation experience with DWARF in 
> general, and with any feature we haven't implemented yet in particular - that 
> I wouldn't throw out their representational choice too quickly - there may be 
> important tradeoffs that were considered, implemented, and discarded due to 
> limitations.
>
>> I'll have to check with my debugger guys whether this would work for us; and 
>> I'll just reiterate that it's certainly not what the DWARF spec suggests 
>> should be done.
>
> I don't agree that it's "certainly not what the DWARF spec suggests should be 
> done" - the spec's pretty generous and exactly how a C or ELF level alias 
> should be rendered in DWARF seems pretty unclear to me. For instance an alias 
> is a symbol in the ELF file, arguably different from a source level alias 
> like a typedef or using declaration that DW_TAG_imported_declaration seems to 
> be promoted for.
>
> I doubt gdb would have trouble with DW_TAG_imported_declaration
>
> In D103131#2791373 , @probinson 
> wrote:
>
>> In D103131#2789493 , @probinson 
>> wrote:
>>
 Mixed feelings - somewhat in favor of "do the thing that's probably 
 already fairly tested/known to work" (GCC's thing). But open to the idea 
 that that approach has problems, for sure.
>>>
>>> "Known to work" for whom?  gdb, sure, because gcc/gdb cooperate on many 
>>> things.  I'll have to check with my debugger guys whether this would work 
>>> for us; and I'll just reiterate that it's certainly not what the DWARF spec 
>>> suggests should be done.
>>
>> The Sony debugger will not be able to figure out the address of "newname" 
>> given the DWARF as emitted by gcc (because it looks like an external 
>> declaration with no address).  We'd much rather have the 
>> DW_TAG_imported_declaration that the original patch had.
>
> Good to know - any interest in the debugger supporting declarations without 
> addresses? I guess there's no need for GCC compatibility? Clang might emit 
> them under some circumstances... let's see... Hmm, can't find a case now. But 
> there are cases where it would be desirable (such as when compiling some code 
> at -g0 and some with debug info - when you only have the declaration on the 
> debug info side, and no debug info on the definition side (eg: GCC produces a 
> declaration of 'i' in this code: `extern int i; int main() { return i; }`))
>
> @kamleshbhalui  - perhaps you could run the gdb and lldb test suites without 
> either change, then with both the variable declaration and imported 
> declaration versions to see how the results vary? (Well, I guess the lldb 
> test suite probably won't show any changes - but maybe worth running anyway) 
> - along with the manual test you've described in the patch description, to 
> demonstrate that both solutions address that?

@dblaikie I have ran gdb and lldb regression here is details.Please let me know 
if we should go to original fix(imported decl).

**case 1) Without any change in clang:**

 

| === gdb Summary === |
|



1. of expected passes75531
2. of unexpected failures695
3. of expected failures  125
4. of known failures 74
5. of unresolved testcases   7
6. of untested testcases 98
7. of unsupported tests  326
8. of duplicate test names   202

| ===lldb summary=== |
|

Failed Tests (3):

  lldb-shell :: Breakpoint/jit-loader_jitlink_elf.test
  lldb-shell :: Breakpoint/jit-loader_rtdyld_elf.test
  lldb-shell :: Reproducer/TestDiscard.test

Testing Time: 60.57s

  Unsupported: 1095
  Passed : 1202
  Failed :3

|
|

**case 2) with imported decl change in clang**

| === gdb Summary === |
|



1. of expected passes75531
2. of unexpected failures695
3. of expected failures  125
4. of known failures 74
5. of unresolved testcases   7
6. of untested testcases 98
7. of unsupported tests  326
8. of duplicate test names   202

| ===lldb summary=== |
|

Failed Tests (3):

  lldb-shell :: Breakpoint/jit-loader_jitlink_elf.test
  lldb-shell :: Breakpoint/jit-loader_rtdyld_elf.test
  lldb-shell 

[PATCH] D103633: [analyzer] Refactor trackExpressionValue to accept TrackingOptions

2021-06-09 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:288
+  bugreporter::trackExpressionValue(
+  N, RS->getRetValue(), *R, {bugreporter::TrackingKind::Thorough, false});
   C.emitReport(std::move(R));

I think `false` deserves a `/*comment=*/`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103633

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


[PATCH] D103630: [analyzer] Refactor trackRValueExpression into ExpressionHandler

2021-06-09 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

I guess you should mark all of these commits as NFC?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103630

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


[PATCH] D103628: [analyzer] Turn ReturnVisitor into a tracking visitor

2021-06-09 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103628

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


[PATCH] D103624: [analyzer] Hide and rename FindLastStoreBRVisitor

2021-06-09 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Excellent!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103624

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


[PATCH] D103618: [analyzer] Change FindLastStoreBRVisitor to use Tracker

2021-06-09 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2274
+   const StackFrameContext *Origin) {
+  Tracker::create(Report)->track(V, R, Opts, Origin);
+}

How does lifetime work here? Do I understand correctly that the tracker is only 
kept alive by the `FindLastStoreBRVisitor` instance, even after its completion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103618

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


[PATCH] D103616: [analyzer] Reimplement trackExpressionValue as ExpressionHandler

2021-06-09 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Everything's perfect, can't complain!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103616

___
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

2021-06-09 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:160-161
+  PathSensitiveBugReport 
+  std::deque ExpressionHandlers;
+  std::deque StoreHandlers;
+

Empty deques can be pretty large:

> https://en.cppreference.com/w/cpp/container/deque
> e.g. 8 times the object size on 64-bit libstdc++; 16 times the object size or 
> 4096 bytes, whichever is larger, on 64-bit libc++

If we're attaching multiple trackers per non-deduplicated bug report I suspect 
this can easily amount to like 20-30 MB per process.

I suspect that a list would be cheaper in every way, esp. given that we don't 
need random access.



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

vsavchenko wrote:
> NoQ wrote:
> > 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.
> +1 for not caring about `UnknownVal`.
That said, I'd rather not force the caller to perform a `getAs<>()`. It's 
pretty cumbersome and we can do the right thing (ignore it) in the callee 
anyway.


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] D103605: [analyzer] Introduce a new interface for tracking

2021-06-09 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.



> - ... Implementation of isTracked on it's own is fairly simple. And we can 
> have users of `isTracked` and `ExpressionHandler` at the same time. Maybe, 
> once we migrate all uses, `ExpressionHandler` can be safely retired.

I think this sounds like a plan. More specifically, if we could maintain a 
//map// of all currently tracked expressions inside the 
`PathSensitiveBugReport` object, dynamically updated as tracking proceeds 
(expressions added as they're tracked and removed as soon as they're no longer 
necessary), with values in the map being arbitrary auxiliary information we 
want to pass around (eg., an explanation of how to refer to the value in 
notes), then all tracking machineries, regardless of how they're implemented, 
could communicate to each other easily through that map.

>> I still think the solution that relies on well-defined end of tracking is 
>> significantly more elegant. Which, again, could be implemented with note 
>> tags: we could have a note tag that'd call a callback stuffed directly into 
>> the bug report.
>
> I'm not sure I understood the last thing, can you please elaborate?

I was thinking of something like:

  void NullDereferenceChecker::checkLocation(...) {
trackExpressionValue(
BR, Ex, /*completionHandler=*/ [](const ExplodedNode *N) {
  if (N->getLocationContext()->inTopFrame())
BR.markInvalid();
}); // stashes completionHandler into the aforementioned tracking map
// inside BR for Ex.
  }
  
  void ExprEngine::processBranch(...) {
Bldr.addTransition(..., getNoteTag([](BugReport ) {
  if (BR.isTracked(Condition)) {
// We've tracked a null pointer back to the assumption.
// Our job here is done. Look up the callback in the tracking map.
if (auto completionHandler = BR.getTrackingCompletionHandler(Condition))
  (*completionHandler)();
  }
}));
  }



> - Why do we even need to introduce tracker and event handlers if we are going 
> to hide/delete them in this plan? The trick is about making this whole plan 
> manageable. One enormous refactoring is hard to plan and test. Big work can 
> demotivate and drain if you don't see the end of it. I believe that we can 
> make great things if we take a lot of baby steps.

I completely agree. Incremental development ftw! I guess I'll keep an eye on 
whether we could convert some of your newly extracted handlers to note tags 
immediately (but most will probably require the expression tracking map to be 
implemented first).

>> This code could also be moved into a checker's `checkBranchCondition()` so 
>> that to make it truly checker-specific. That would come at the cost of 
>> creating a lot of redundant exploded nodes just for the sake of attaching 
>> the tag that will probably never be used so this is rather bad, wouldn't 
>> recommend.
>
> Why not model defensive checks in the checker, so we don't make undesired 
> assumptions in the first place?

That's pretty difficult. We can't enter (or skip) a branch without recording 
its respective condition. We can't continue analysis unless we enter (or skip) 
the branch. Once the branch has exited, we can't get rid of the constraint yet 
because we may encounter the same condition again and we're obliged to be 
consistent. Even when we've exited the nested stack frame we're still obliged 
to be consistent, unless we're also willing to completely drop the execution 
path inside the function and replace it with some form of summary. Or, you 
know, do summaries from the start.



Ok, I think I mostly understand where this is going, thanks!

I'll now dive into code.


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] D103426: Clang: Extend format string checking to wprintf/wscanf

2021-06-09 Thread Marcus Johnson via Phabricator via cfe-commits
MarcusJohnson91 added a comment.

In D103426#2807860 , @aaron.ballman 
wrote:

> In D103426#2807245 , 
> @MarcusJohnson91 wrote:
>
>> In D103426#2806391 , 
>> @aaron.ballman wrote:
>>
>>> Do you have a reference to the WG14 paper proposing these conversion 
>>> specifiers?
>>
>> I've previously written up the proposal, which you actually helped with 
>> (thank you for that), just submitted a request to Dan Plakosh to get a 
>> document number and submit it.
>
> Thanks! Once it's submitted and available on the committee webpage, can you 
> post a link to it in this review? That'd help assess whether the 
> implementation matches the proposal or not.

Absolutely :)

>> Maybe I was a bit optimistic, but I don't think l16 and l32 will be a hard 
>> sell, but maybe I'm naive haha
>
> There's never a shortage of surprises when it comes to standardization 
> efforts. :-D

lol, well I'm hopeful it'll be smooth sailing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103426

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


[PATCH] D103967: [Analyzer][solver] Add dump methods for (dis)equality classes.

2021-06-09 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Yes we should definitely have as much as possible in the state dump.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103967

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


[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

2021-06-09 Thread Marcus Johnson via Phabricator via cfe-commits
MarcusJohnson91 added inline comments.



Comment at: clang/lib/AST/Type.cpp:1980-2002
+bool Type::isChar16Type(const LangOptions ) const {
   if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Char16;
-  return false;
-}
-
-bool Type::isChar32Type() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Char32;
-  return false;
-}
-
-/// Determine whether this type is any of the built-in character
-/// types.
-bool Type::isAnyCharacterType() const {
-  const auto *BT = dyn_cast(CanonicalType);
-  if (!BT) return false;
-  switch (BT->getKind()) {
-  default: return false;
-  case BuiltinType::Char_U:
-  case BuiltinType::UChar:
-  case BuiltinType::WChar_U:
-  case BuiltinType::Char8:
-  case BuiltinType::Char16:
-  case BuiltinType::Char32:
-  case BuiltinType::Char_S:
-  case BuiltinType::SChar:
-  case BuiltinType::WChar_S:
-return true;
+if (BT->getKind() == BuiltinType::Char16)
+  return true;
+  if (!LangOpts.CPlusPlus) {
+return isType("char16_t");
   }

aaron.ballman wrote:
> If I understand properly, one issue is that `char16_t` is defined by C to be 
> *the same type* as `uint_least16_t`, which itself is a typedef to some other 
> integer type. So we can't make `char16_t` be a distinct type in C, it has to 
> be a typedef to a typedef to an integer type.
> 
> One concern I have about the approach in this patch is that it makes the type 
> system a bit more convoluted. This type is *not* really a character type in 
> C, it's a typedef type that playacts as a character type. I think this is a 
> pretty fundamental change to the type system in the compiler in some ways 
> because this query is no longer about the canonical type but about the 
> semantics of how the type is expected to be used.
> 
> I'd definitely like to hear what @rsmith thinks of this approach.
I see your point, but I'm not sure how else we could get it through the type 
checking system without doing it this way?

I tried to be careful to only allow the actual character typedefs through by 
making sure char16_t or char32_t is in the typedef chain, and only in C mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103426

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


[PATCH] D101630: [HIP] Add --gpu-bundle-output

2021-06-09 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rG5fc2673fbce2: [HIP] Add --gpu-bundle-output (authored by 
yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D101630?vs=350318=351053#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101630

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/clang-offload-bundler.c
  clang/test/Driver/hip-device-compile.hip
  clang/test/Driver/hip-output-file-name.hip
  clang/test/Driver/hip-phases.hip
  clang/test/Driver/hip-rdc-device-only.hip
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -117,6 +117,9 @@
 /// The index of the host input in the list of inputs.
 static unsigned HostInputIndex = ~0u;
 
+/// Whether not having host target is allowed.
+static bool AllowNoHost = false;
+
 /// Path to the current binary.
 static std::string BundlerExecutable;
 
@@ -839,9 +842,10 @@
   }
 
   // Get the file handler. We use the host buffer as reference.
-  assert(HostInputIndex != ~0u && "Host input index undefined??");
+  assert((HostInputIndex != ~0u || AllowNoHost) &&
+ "Host input index undefined??");
   Expected> FileHandlerOrErr =
-  CreateFileHandler(*InputBuffers[HostInputIndex]);
+  CreateFileHandler(*InputBuffers[AllowNoHost ? 0 : HostInputIndex]);
   if (!FileHandlerOrErr)
 return FileHandlerOrErr.takeError();
 
@@ -1108,6 +1112,7 @@
   // have exactly one host target.
   unsigned Index = 0u;
   unsigned HostTargetNum = 0u;
+  bool HIPOnly = true;
   llvm::DenseSet ParsedTargets;
   for (StringRef Target : TargetNames) {
 if (ParsedTargets.contains(Target)) {
@@ -1149,12 +1154,21 @@
   HostInputIndex = Index;
 }
 
+if (Kind != "hip" && Kind != "hipv4")
+  HIPOnly = false;
+
 ++Index;
   }
 
+  // HIP uses clang-offload-bundler to bundle device-only compilation results
+  // for multiple GPU archs, therefore allow no host target if all entries
+  // are for HIP.
+  AllowNoHost = HIPOnly;
+
   // Host triple is not really needed for unbundling operation, so do not
   // treat missing host triple as error if we do unbundling.
-  if ((Unbundle && HostTargetNum > 1) || (!Unbundle && HostTargetNum != 1)) {
+  if ((Unbundle && HostTargetNum > 1) ||
+  (!Unbundle && HostTargetNum != 1 && !AllowNoHost)) {
 reportError(createStringError(errc::invalid_argument,
   "expecting exactly one host target but got " +
   Twine(HostTargetNum)));
Index: clang/test/Driver/hip-rdc-device-only.hip
===
--- clang/test/Driver/hip-rdc-device-only.hip
+++ clang/test/Driver/hip-rdc-device-only.hip
@@ -6,7 +6,7 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -c -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITBC %s
 
 // With `-emit-llvm`, the output should be the same as the aforementioned line
@@ -16,14 +16,14 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -c -emit-llvm -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITBC %s
 
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -S -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITLL %s
 
 // With `-emit-llvm`, the output should be the same as the aforementioned line
@@ -33,7 +33,7 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -S -emit-llvm -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITLL %s
 
 // With `-save-temps`, commane lines for each steps are dumped. For assembly
@@ -44,9 +44,17 @@
 // RUN:   -x 

[clang] 5fc2673 - [HIP] Add --gpu-bundle-output

2021-06-09 Thread Yaxun Liu via cfe-commits

Author: Yaxun (Sam) Liu
Date: 2021-06-09T23:31:43-04:00
New Revision: 5fc2673fbce247e107094b28c22cbb2d5f1691a8

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

LOG: [HIP] Add --gpu-bundle-output

Added --gpu-bundle-output to control bundling/unbundling output of HIP device 
compilation.

By default preprocessor expansion, llvm bitcode and assembly are unbundled, 
code objects are
bundled.

Reviewed by: Artem Belevich, Jan Svoboda

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/Driver.cpp
clang/test/Driver/clang-offload-bundler.c
clang/test/Driver/hip-device-compile.hip
clang/test/Driver/hip-output-file-name.hip
clang/test/Driver/hip-phases.hip
clang/test/Driver/hip-rdc-device-only.hip
clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 55391cf2dac1d..7dcee76b4ed80 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -988,6 +988,10 @@ def gpu_instrument_lib_EQ : Joined<["--"], 
"gpu-instrument-lib=">,
 def fgpu_sanitize : Flag<["-"], "fgpu-sanitize">, Group,
   HelpText<"Enable sanitizer for AMDGPU target">;
 def fno_gpu_sanitize : Flag<["-"], "fno-gpu-sanitize">, Group;
+def gpu_bundle_output : Flag<["--"], "gpu-bundle-output">,
+  Group, HelpText<"Bundle output files of HIP device compilation">;
+def no_gpu_bundle_output : Flag<["--"], "no-gpu-bundle-output">,
+  Group, HelpText<"Do not bundle output files of HIP device 
compilation">;
 def cuid_EQ : Joined<["-"], "cuid=">, Flags<[CC1Option]>,
   HelpText<"An ID for compilation unit, which should be the same for the same "
"compilation unit but 
diff erent for 
diff erent compilation units. "

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index cd2c8c9b19165..930941fe8558c 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2907,6 +2907,12 @@ class OffloadingActionBuilder final {
 /// The linker inputs obtained for each device arch.
 SmallVector DeviceLinkerInputs;
 bool GPUSanitize;
+// The default bundling behavior depends on the type of output, therefore
+// BundleOutput needs to be tri-value: None, true, or false.
+// Bundle code objects except --no-gpu-output is specified for device
+// only compilation. Bundle other type of output files only if
+// --gpu-bundle-output is specified for device only compilation.
+Optional BundleOutput;
 
   public:
 HIPActionBuilder(Compilation , DerivedArgList ,
@@ -2915,6 +2921,10 @@ class OffloadingActionBuilder final {
   DefaultCudaArch = CudaArch::GFX803;
   GPUSanitize = Args.hasFlag(options::OPT_fgpu_sanitize,
  options::OPT_fno_gpu_sanitize, false);
+  if (Args.hasArg(options::OPT_gpu_bundle_output,
+  options::OPT_no_gpu_bundle_output))
+BundleOutput = Args.hasFlag(options::OPT_gpu_bundle_output,
+options::OPT_no_gpu_bundle_output);
 }
 
 bool canUseBundlerUnbundler() const override { return true; }
@@ -3004,22 +3014,25 @@ class OffloadingActionBuilder final {
   CudaDeviceActions[I] = C.MakeAction(
   DDep, CudaDeviceActions[I]->getType());
 }
-// Create HIP fat binary with a special "link" action.
-CudaFatBinary =
-C.MakeAction(CudaDeviceActions,
-types::TY_HIP_FATBIN);
 
-if (!CompileDeviceOnly) {
-  DA.add(*CudaFatBinary, *ToolChains.front(), /*BoundArch=*/nullptr,
- AssociatedOffloadKind);
-  // Clear the fat binary, it is already a dependence to an host
-  // action.
-  CudaFatBinary = nullptr;
-}
+if (!CompileDeviceOnly || !BundleOutput.hasValue() ||
+BundleOutput.getValue()) {
+  // Create HIP fat binary with a special "link" action.
+  CudaFatBinary = C.MakeAction(CudaDeviceActions,
+  types::TY_HIP_FATBIN);
 
-// Remove the CUDA actions as they are already connected to an host
-// action or fat binary.
-CudaDeviceActions.clear();
+  if (!CompileDeviceOnly) {
+DA.add(*CudaFatBinary, *ToolChains.front(), /*BoundArch=*/nullptr,
+   AssociatedOffloadKind);
+// Clear the fat binary, it is already a dependence to an host
+// action.
+CudaFatBinary = nullptr;
+  }
+
+  // Remove the CUDA actions as they are already connected to an host
+  // action 

[PATCH] D95425: Implementation of global.get/set for reftypes in LLVM IR

2021-06-09 Thread Thomas Lively via Phabricator via cfe-commits
tlively accepted this revision.
tlively added a comment.
This revision is now accepted and ready to land.

Thanks for your patience through all the review! Let's get this landed :)




Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:19
  [],
  "table.get\t$res, $table",
  "table.get\t$table",

pmatos wrote:
> tlively wrote:
> > pmatos wrote:
> > > tlively wrote:
> > > > wingo wrote:
> > > > > Not something for this patch, but this is certainly bogus: surely we 
> > > > > mean `table.get\t$table, $i` and we have an i32 index argument?
> > > > Agree about the i32 index argument, but it is correct to have the 
> > > > result as part of the string for the register-based output format.
> > > Not sure I understand why this should be `$i` instead of `$table`. Isn't 
> > > `$table` represented as an index anyway? A `table32_op` is defined as 
> > > `Operand` so I don't quite understand what we gain by changing this.
> > I would still expect to see a register for the result and immediate inputs 
> > for the table symbol and the table slot index here.
> Not sure I understood everything properly as the only thing missing was the 
> index in the register based version. I added that now.
Ah, sorry about that. I was confused and was thinking that the table index was 
an immediate, but of course you're right that it is not.



Comment at: llvm/test/CodeGen/WebAssembly/funcref-call.ll:15
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: table.set __funcref_call_table
+; CHECK-NEXT: local.get 0

pmatos wrote:
> tlively wrote:
> > Missing a table element index here.
> I am not sure whether that's the case. According to the spec, table.set only 
> gets a table. Two elements are popped from the stack: the index `i32.const 0` 
> and the value `local.get 0`.
Yep, sorry for the confusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95425

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


[PATCH] D103894: NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

2021-06-09 Thread Stephen Concannon via Phabricator via cfe-commits
Stephen updated this revision to Diff 351030.
Stephen added a comment.

Try again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103894

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -11,16 +11,35 @@
 
 void narrowing_equivalent_bitwidth() {
   int i;
-  unsigned int j;
-  i = j;
+  unsigned int ui;
+  i = ui;
   // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  float f;
+  i = f;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  f = i;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  long long ll;
+  double d;
+  ll = d;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  d = ll;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 
 void most_narrowing_is_not_ok() {
   int i;
-  long long j;
-  i = j;
+  long long ui;
+  i = ui;
   // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
   // CHECK-MESSAGES-DISABLED: :[[@LINE-2]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
 }
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
@@ -93,6 +93,10 @@
   void handleBinaryOperator(const ASTContext ,
 const BinaryOperator );
 
+  bool isWarningInhibitedByEquivalentSize(const ASTContext ,
+  const BuiltinType ,
+  const BuiltinType ) const;
+
   const bool WarnOnFloatingPointNarrowingConversion;
   const bool WarnWithinTemplateInstantiation;
   const bool WarnOnEquivalentBitWidth;
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -67,8 +67,9 @@
   hasType(namedDecl(hasAnyListedName(IgnoreConversionFromTypes)));
 
   // `IsConversionFromIgnoredType` will ignore narrowing calls from those types,
-  // but not expressions that are promoted to `int64` due to a binary expression
-  // with one of those types. For example, it will continue to reject:
+  // but not expressions that are promoted to an ignored type as a result of a
+  // binary expression with one of those types.
+  // For example, it will continue to reject:
   // `int narrowed = int_value + container.size()`.
   // We attempt to address common incidents of compound expressions with
   // `IsIgnoredTypeTwoLevelsDeep`, allowing binary expressions that have one
@@ -217,6 +218,22 @@
   return ToIntegerRange.contains(IntegerConstant);
 }
 
+// Returns true iff the floating point constant can be losslessly represented
+// by an integer in the given destination type. eg. 2.0 can be accurately
+// represented by an int32_t, but neither 2^33 nor 2.001 can.
+static bool isFloatExactlyRepresentable(const ASTContext ,
+const llvm::APFloat ,
+const QualType ) {
+  unsigned 

[PATCH] D103894: NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

2021-06-09 Thread Stephen Concannon via Phabricator via cfe-commits
Stephen updated this revision to Diff 351029.
Stephen added a comment.

Try specifying the commit range.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103894

Files:
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to signed type 'int' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to signed type 'float' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to signed type 'long long' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to signed type 'double' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to signed type 'float' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to signed type 'long long' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'double' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103894: NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

2021-06-09 Thread Stephen Concannon via Phabricator via cfe-commits
Stephen updated this revision to Diff 351028.
Stephen added a comment.

Undo that last move, since that didn't work.
I'm absolutely incompetent with this tool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103894

Files:
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to signed type 'int' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to signed type 'float' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to signed type 'long long' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to signed type 'double' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to signed type 'float' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to signed type 'long long' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'double' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103878: [clang][RISCV][test] Add more tests of the -mabi and -march options

2021-06-09 Thread Ben Shi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb0eb3919835a: [clang][RISCV][test] Add more tests of the 
-mabi and -march options (authored by benshi001).

Changed prior to commit:
  https://reviews.llvm.org/D103878?vs=350828=351025#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103878

Files:
  clang/test/Driver/riscv-abi.c
  clang/test/Driver/riscv-arch.c

Index: clang/test/Driver/riscv-arch.c
===
--- clang/test/Driver/riscv-arch.c
+++ clang/test/Driver/riscv-arch.c
@@ -39,6 +39,33 @@
 // RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
 
+// RUN: %clang -target riscv32-unknown-elf -mabi=ilp32 -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ILP32 %s
+
+// CHECK-ILP32:  "-target-feature" "+m"
+// CHECK-ILP32-SAME: {{^}} "-target-feature" "+a"
+// CHECK-ILP32-SAME: {{^}} "-target-feature" "+f"
+// CHECK-ILP32-SAME: {{^}} "-target-feature" "+d"
+// CHECK-ILP32-SAME: {{^}} "-target-feature" "+c"
+
+// RUN: %clang -target riscv32-unknown-elf -mabi=ilp32f -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ILP32F %s
+
+// CHECK-ILP32F:  "-target-feature" "+m"
+// CHECK-ILP32F-SAME: {{^}} "-target-feature" "+a"
+// CHECK-ILP32F-SAME: {{^}} "-target-feature" "+f"
+// CHECK-ILP32F-SAME: {{^}} "-target-feature" "+d"
+// CHECK-ILP32F-SAME: {{^}} "-target-feature" "+c"
+
+// RUN: %clang -target riscv32-unknown-elf -mabi=ilp32d -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ILP32D %s
+
+// CHECK-ILP32D:  "-target-feature" "+m"
+// CHECK-ILP32D-SAME: {{^}} "-target-feature" "+a"
+// CHECK-ILP32D-SAME: {{^}} "-target-feature" "+f"
+// CHECK-ILP32D-SAME: {{^}} "-target-feature" "+d"
+// CHECK-ILP32D-SAME: {{^}} "-target-feature" "+c"
+
 // RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s \
@@ -80,6 +107,33 @@
 // RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
 
+// RUN: %clang -target riscv64-unknown-elf -mabi=lp64 -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-LP64 %s
+
+// CHECK-LP64: "-target-feature" "+m"
+// CHECK-LP64-SAME: {{^}} "-target-feature" "+a"
+// CHECK-LP64-SAME: {{^}} "-target-feature" "+f"
+// CHECK-LP64-SAME: {{^}} "-target-feature" "+d"
+// CHECK-LP64-SAME: {{^}} "-target-feature" "+c"
+
+// RUN: %clang -target riscv64-unknown-elf -mabi=lp64f -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-LP64F %s
+
+// CHECK-LP64F: "-target-feature" "+m"
+// CHECK-LP64F-SAME: {{^}} "-target-feature" "+a"
+// CHECK-LP64F-SAME: {{^}} "-target-feature" "+f"
+// CHECK-LP64F-SAME: {{^}} "-target-feature" "+d"
+// CHECK-LP64F-SAME: {{^}} "-target-feature" "+c"
+
+// RUN: %clang -target riscv64-unknown-elf -mabi=lp64d -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-LP64D %s
+
+// CHECK-LP64D: "-target-feature" "+m"
+// CHECK-LP64D-SAME: {{^}} "-target-feature" "+a"
+// CHECK-LP64D-SAME: {{^}} "-target-feature" "+f"
+// CHECK-LP64D-SAME: {{^}} "-target-feature" "+d"
+// CHECK-LP64D-SAME: {{^}} "-target-feature" "+c"
+
 // CHECK-NOT: error: invalid arch name '
 
 // RUN: %clang -target riscv32-unknown-elf -march=rv32 -### %s \
Index: clang/test/Driver/riscv-abi.c
===
--- clang/test/Driver/riscv-abi.c
+++ clang/test/Driver/riscv-abi.c
@@ -2,6 +2,10 @@
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32imc 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32imf 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o \
@@ -9,6 +13,15 @@
 
 // CHECK-ILP32: "-target-abi" "ilp32"
 
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32e -mabi=ilp32e 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32E %s
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32e 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32E %s
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32e 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32E %s
+
+// CHECK-ILP32E: "-target-abi" "ilp32e"
+
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32if -mabi=ilp32f 

[clang] b0eb391 - [clang][RISCV][test] Add more tests of the -mabi and -march options

2021-06-09 Thread Ben Shi via cfe-commits

Author: Ben Shi
Date: 2021-06-10T09:14:14+08:00
New Revision: b0eb3919835a7bb57cb28ae684d77fece8ff025c

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

LOG: [clang][RISCV][test] Add more tests of the -mabi and -march options

1. There is no tests for mabi=ilp32e, and my patch covers that.
2. The tests in riscv-abi.c will show default ABI changes for special archs
   in the future, especially the arch with the F but without the D extension.
3. The tests in riscv-arch.c will show default arch changes for abi=ilp32,
   which is rv32imacfd currently, but it is better to be rv32imac.
   And it is also better for abi=ilp32f defaults to arch=imacf.

Reviewed By: MaskRay, luismarques

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

Added: 


Modified: 
clang/test/Driver/riscv-abi.c
clang/test/Driver/riscv-arch.c

Removed: 




diff  --git a/clang/test/Driver/riscv-abi.c b/clang/test/Driver/riscv-abi.c
index ef3913ee3c4b..1035fe4c668b 100644
--- a/clang/test/Driver/riscv-abi.c
+++ b/clang/test/Driver/riscv-abi.c
@@ -2,6 +2,10 @@
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32imc 2>&1 
\
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32imf 2>&1 
\
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o \
@@ -9,6 +13,15 @@
 
 // CHECK-ILP32: "-target-abi" "ilp32"
 
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32e 
-mabi=ilp32e 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32E %s
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32e 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32E %s
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32e 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32E %s
+
+// CHECK-ILP32E: "-target-abi" "ilp32e"
+
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32if 
-mabi=ilp32f 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32F %s
 
@@ -16,6 +29,10 @@
 
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32ifd 
-mabi=ilp32d 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32ifd 2>&1 
\
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32g 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
 // RUN: %clang -target riscv32-unknown-linux-gnu %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
 // RUN: %clang -target riscv32-unknown-linux-gnu -x assembler %s -### -o %t.o 
2>&1 \
@@ -32,6 +49,10 @@
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -mabi=lp64 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
+// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -march=rv64imc 2>&1 
\
+// RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
+// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -march=rv64imf 2>&1 
\
+// RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
 // RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64  %s
 // RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o \
@@ -46,6 +67,10 @@
 
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -march=rv64d 
-mabi=lp64d 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
+// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -march=rv64d 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
+// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -march=rv64g 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
 // RUN: %clang -target riscv64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -x assembler %s -### -o %t.o 
2>&1 \

diff  --git a/clang/test/Driver/riscv-arch.c b/clang/test/Driver/riscv-arch.c
index cf148ca885d0..f79d29d87e98 100644
--- a/clang/test/Driver/riscv-arch.c
+++ b/clang/test/Driver/riscv-arch.c
@@ -39,6 +39,33 @@
 // RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
 
+// RUN: %clang -target riscv32-unknown-elf -mabi=ilp32 -### %s \
+// RUN: -fsyntax-only 2>&1 

[PATCH] D103894: NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

2021-06-09 Thread Stephen Concannon via Phabricator via cfe-commits
Stephen updated this revision to Diff 351024.
Stephen added a comment.

Fix the diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103894

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=WARN %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: cppcoreguidelines-narrowing-conversions.WarnWithinTemplateInstantiation, value: 1} \
+// RUN: ]}'
+
+template 
+void assign_in_template(OrigType jj) {
+  int ii;
+  ii = jj;
+  // DEFAULT: Warning disabled because WarnWithinTemplateInstantiation=0.
+  // CHECK-MESSAGES-WARN: :[[@LINE-2]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_inside_template_not_ok() {
+  long long j = 123;
+  assign_in_template(j);
+}
+
+void assign_outside_template(long long jj) {
+  int ii;
+  ii = jj;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-WARN: :[[@LINE-2]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_outside_template_not_ok() {
+  long long j = 123;
+  assign_outside_template(j);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=IGNORED %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes, value: "global_size_t;nested_size_type"} \
+// RUN: ]}'
+
+// We use global_size_t instead of 'size_t' because windows predefines size_t.
+typedef long long global_size_t;
+
+struct vector {
+  typedef long long nested_size_type;
+
+  global_size_t size() const { return 0; }
+};
+
+void narrowing_global_size_t() {
+  int i;
+  global_size_t j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'global_size_t' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=global_size_t.
+}
+
+void narrowing_size_type() {
+  int i;
+  vector::nested_size_type j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'vector::nested_size_type' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=nested_size_type.
+}
+
+void narrowing_size_method() {
+  vector v;
+  int i, j;
+  i = v.size();
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'global_size_t' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=global_size_t.
+
+  i = j + v.size();
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=global_size_t.
+}
+
+void 

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

2021-06-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

You could break `__builtin_load_with_control_dependency(x)` into something like 
`__builtin_control_dependency(READ_ONCE(x))`.  I don't think any transforms 
will touch that in practice, even if it isn't theoretically sound.  The rest of 
my suggestion still applies to that form, I think.  They key point is that the 
compiler just needs to ensure some branch consumes the loaded value; it doesn't 
matter which branch it is.

The theoretical problem with separating the load from the branch is that it 
imposes an implicit contract: the branch has to use the value of the load as 
input, not an equivalent value produced some other way.  This is the general 
problem with C++11 consume ordering, which nobody has tried to tackle.

re: the memory clobber, LLVM understands acquire/release semantics for atomics; 
for example, you can write `__atomic_signal_fence(3)` in clang to get a 
"release" barrier.  (I think that's what the email you linked is asking for?)  
Adding asm clobbers that are equivalent to the existing fences is probably 
feasible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103958

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


[PATCH] D102875: [PowerPC] Add PowerPC compare and multiply related builtins and instrinsics for XL compatibility

2021-06-09 Thread Victor Huang via Phabricator via cfe-commits
NeHuang updated this revision to Diff 350991.
NeHuang added a comment.

- Renamed the XLCompat builtin as `__builtin_ppc_*` and add them to 
`definedXLCompatMacros` and update the test cases.
- Report error in SemaChecking when 64 bit only builtins run on a 32 bit target 
and update the test cases.
- Move the XLCompat Intrinsics definition under

  let TargetPrefix = "ppc" in {
  ...
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102875

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-ppc-xlcompat-compare-64bit-only.c
  clang/test/CodeGen/builtins-ppc-xlcompat-compare.c
  clang/test/CodeGen/builtins-ppc-xlcompat-multiply-64bit-only.c
  clang/test/CodeGen/builtins-ppc-xlcompat-multiply.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstr64Bit.td
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-compare-64bit-only.ll
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-compare.ll
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply-64bit-only.ll
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply.ll
@@ -0,0 +1,52 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpcle-unknown-linux-gnu \
+; RUN: -mcpu=pwr9 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | FileCheck %s --check-prefix=CHECK-32
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-linux-gnu \
+; RUN: -mcpu=pwr9 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | FileCheck %s --check-prefix=CHECK-32
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN: -mcpu=pwr9 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | FileCheck %s --check-prefix=CHECK-64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN: -mcpu=pwr9 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | FileCheck %s --check-prefix=CHECK-64
+
+; Function Attrs: noinline nounwind optnone
+define dso_local signext i32 @test_builtin_ppc_mulhw(i32 %a, i32%b) #0 {
+; CHECK-32-LABEL: test_builtin_ppc_mulhw:
+; CHECK-32:   # %bb.0: # %entry
+; CHECK-32-NEXT:mulhw r3, r3, r4
+; CHECK-32-NEXT:blr
+;
+; CHECK-64-LABEL: test_builtin_ppc_mulhw:
+; CHECK-64:   # %bb.0: # %entry
+; CHECK-64-NEXT:mulhw r3, r3, r4
+; CHECK-64-NEXT:extsw r3, r3
+; CHECK-64-NEXT:blr
+entry:
+  %0 = call i32 @llvm.ppc.mulhw(i32 %a, i32 %b)
+  ret i32 %0
+}
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.ppc.mulhw(i32, i32) #1
+
+; Function Attrs: noinline nounwind optnone
+define dso_local zeroext i32 @test_builtin_ppc_mulhwu(i32 %a, i32%b) #0 {
+; CHECK-32-LABEL: test_builtin_ppc_mulhwu:
+; CHECK-32:   # %bb.0: # %entry
+; CHECK-32-NEXT:mulhwu r3, r3, r4
+; CHECK-32-NEXT:blr
+;
+; CHECK-64-LABEL: test_builtin_ppc_mulhwu:
+; CHECK-64:   # %bb.0: # %entry
+; CHECK-64-NEXT:mulhwu r3, r3, r4
+; CHECK-64-NEXT:clrldi r3, r3, 32
+; CHECK-64-NEXT:blr
+entry:
+  %0 = call i32 @llvm.ppc.mulhwu(i32 %a, i32 %b)
+  ret i32 %0
+}
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.ppc.mulhwu(i32, i32) #1
+
+attributes #0 = { noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="pwr9" "target-features"="+altivec,+bpermd,+crypto,+direct-move,+extdiv,+htm,+power8-vector,+power9-vector,+vsx,-privileged,-rop-protect,-spe" }
+attributes #1 = { nounwind readnone }
Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply-64bit-only.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply-64bit-only.ll
@@ -0,0 +1,80 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN: -mcpu=pwr9 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN: -mcpu=pwr9 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | FileCheck %s
+
+; Function Attrs: noinline nounwind optnone
+define dso_local i64 @test_builtin_ppc_mulhd(i64 %a, i64 %b) #0 {
+; CHECK-LABEL: test_builtin_ppc_mulhd:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mulhd r3, r3, r4
+; CHECK-NEXT:blr
+entry:
+  %0 = call i64 @llvm.ppc.mulhd(i64 %a, i64 %b)
+  ret i64 %0
+}
+
+; Function Attrs: nounwind readnone
+declare i64 @llvm.ppc.mulhd(i64, i64) #1
+
+; Function Attrs: noinline nounwind optnone
+define dso_local i64 

[PATCH] D103252: [C++4OpenCL] Fix missing address space on implicit move assignment operator

2021-06-09 Thread Abhinav Gaba via Phabricator via cfe-commits
abhinavgaba added inline comments.



Comment at: clang/test/AST/ast-dump-implicit-members.clcpp:10
+
+// CHECK: CXXConstructorDecl {{.*}} implicit used constexpr S 'void () 
__generic noexcept'
+// CHECK: CXXConstructorDecl {{.*}} implicit constexpr S 'void (const 
__generic S &) __generic'

Could you please relax these checks? The test fails with `-triple 
i386-pc-win32`  with errors like this:
```
ast-dump-implicit-members.clcpp:10:11: error: CHECK: expected string not found 
in input
// CHECK: CXXConstructorDecl {{.*}} implicit used constexpr S 'void () 
__generic noexcept'
  ^
:1:1: note: scanning from here
Dumping __NSConstantString:
^
:16:18: note: possible intended match here
|-CXXConstructorDecl 0x10b6c718  col:8 implicit used constexpr S 'void 
() __attribute__((thiscall)) __generic noexcept' inline default trivial
 ^
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103252

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


[PATCH] D103995: [OpenMP] Add type to firstprivate symbol for const firstprivate values

2021-06-09 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 351012.
jhuber6 added a comment.

Changing to only mangle when using C++. It should be correct either way but is 
unnecessary with C.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103995

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp


Index: clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
===
--- clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
+++ clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
@@ -13,11 +13,14 @@
   ty Y;
 };
 
-// TCHECK-DAG:  [[TT:%.+]] = type { i64, i8 }
 // TCHECK-DAG:  [[TTII:%.+]] = type { i32, i32 }
+// TCHECK-DAG:  [[TTIC:%.+]] = type { i8, i8 }
+// TCHECK-DAG:  [[TT:%.+]] = type { i64, i8 }
 // TCHECK-DAG:  [[S1:%.+]] = type { double }
 
-// TCHECK: @__omp_offloading_firstprivate__{{.+}}_e_l27 = internal 
addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_e_l30 = internal 
addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_ZTSK2TTIiiE_t_l143 = 
internal addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_ZTSK2TTIccE_t_l143 = 
internal addrspace(4) global [[TTIC]] zeroinitializer
 int foo(int n, double *ptr) {
   int a = 0;
   short aa = 0;
@@ -136,6 +139,12 @@
   return a;
 }
 
+template 
+void fconst(const tx t) {
+#pragma omp target firstprivate(t)
+  { }
+}
+
 // TCHECK: define {{.*}}void @__omp_offloading_{{.+}}(i{{[0-9]+}}{{.*}} 
[[A_IN:%.+]], i{{[0-9]+}}{{.*}} [[A3_IN:%.+]], [10 x i{{[0-9]+}}]*{{.+}} 
[[B_IN:%.+]])
 // TCHECK:  [[A_ADDR:%.+]] = alloca i{{[0-9]+}},
 // TCHECK:  [[A3_ADDR:%.+]] = alloca i{{[0-9]+}},
@@ -199,6 +208,9 @@
   a += fstatic(n);
   a += ftemplate(n);
 
+  fconst(TT{0, 0});
+  fconst(TT{0, 0});
+
   return a;
 }
 
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -10593,7 +10593,12 @@
  FileID, Line);
 llvm::raw_svector_ostream OS(Buffer);
 OS << "__omp_offloading_firstprivate_" << llvm::format("_%x", DeviceID)
-   << llvm::format("_%x_", FileID) << VD->getName() << "_l" << Line;
+   << llvm::format("_%x_", FileID);
+if (CGM.getLangOpts().CPlusPlus) {
+  CGM.getCXXABI().getMangleContext().mangleTypeName(VD->getType(), OS);
+  OS << "_";
+}
+OS << VD->getName() << "_l" << Line;
 VarName = OS.str();
   }
   Linkage = llvm::GlobalValue::InternalLinkage;


Index: clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
===
--- clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
+++ clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
@@ -13,11 +13,14 @@
   ty Y;
 };
 
-// TCHECK-DAG:  [[TT:%.+]] = type { i64, i8 }
 // TCHECK-DAG:  [[TTII:%.+]] = type { i32, i32 }
+// TCHECK-DAG:  [[TTIC:%.+]] = type { i8, i8 }
+// TCHECK-DAG:  [[TT:%.+]] = type { i64, i8 }
 // TCHECK-DAG:  [[S1:%.+]] = type { double }
 
-// TCHECK: @__omp_offloading_firstprivate__{{.+}}_e_l27 = internal addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_e_l30 = internal addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_ZTSK2TTIiiE_t_l143 = internal addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_ZTSK2TTIccE_t_l143 = internal addrspace(4) global [[TTIC]] zeroinitializer
 int foo(int n, double *ptr) {
   int a = 0;
   short aa = 0;
@@ -136,6 +139,12 @@
   return a;
 }
 
+template 
+void fconst(const tx t) {
+#pragma omp target firstprivate(t)
+  { }
+}
+
 // TCHECK: define {{.*}}void @__omp_offloading_{{.+}}(i{{[0-9]+}}{{.*}} [[A_IN:%.+]], i{{[0-9]+}}{{.*}} [[A3_IN:%.+]], [10 x i{{[0-9]+}}]*{{.+}} [[B_IN:%.+]])
 // TCHECK:  [[A_ADDR:%.+]] = alloca i{{[0-9]+}},
 // TCHECK:  [[A3_ADDR:%.+]] = alloca i{{[0-9]+}},
@@ -199,6 +208,9 @@
   a += fstatic(n);
   a += ftemplate(n);
 
+  fconst(TT{0, 0});
+  fconst(TT{0, 0});
+
   return a;
 }
 
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -10593,7 +10593,12 @@
  FileID, Line);
 llvm::raw_svector_ostream OS(Buffer);
 OS << "__omp_offloading_firstprivate_" << llvm::format("_%x", DeviceID)
-   << llvm::format("_%x_", FileID) << VD->getName() << "_l" << Line;
+   << llvm::format("_%x_", FileID);
+if (CGM.getLangOpts().CPlusPlus) {
+  CGM.getCXXABI().getMangleContext().mangleTypeName(VD->getType(), OS);
+  OS << "_";
+}
+

[PATCH] D103995: [OpenMP] Add type to firstprivate symbol for const firstprivate values

2021-06-09 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10598
+CGM.getCXXABI().getMangleContext().mangleTypeName(VD->getType(), OS);
+OS << "_" << VD->getName() << "_l" << Line;
 VarName = OS.str();

jdoerfert wrote:
> What if the code is not C++?
Should work fine, this just mangles it according to the C++ ABI but doesn't 
require it being C++ as far as I know. Testing it with a sample C file creates 
the same symbol name as the C++ version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103995

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


[PATCH] D103995: [OpenMP] Add type to firstprivate symbol for const firstprivate values

2021-06-09 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10598
+CGM.getCXXABI().getMangleContext().mangleTypeName(VD->getType(), OS);
+OS << "_" << VD->getName() << "_l" << Line;
 VarName = OS.str();

What if the code is not C++?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103995

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


[PATCH] D103878: [clang][RISCV][test] Add more tests of the -mabi and -march options

2021-06-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: clang/test/Driver/riscv-arch.c:122
+
+// CHECK-LP64F: "-target-feature" "+m"
+// CHECK-LP64F-SAME: {{^}} "-target-feature" "+a"

```
// CHECK-LP64F:  "-target-feature" "+m"
// CHECK-LP64F-SAME: {{^}} "-target-feature" "+a"
```


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

https://reviews.llvm.org/D103878

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


[PATCH] D103928: [IR] make -warn-frame-size into a module attr

2021-06-09 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 351008.
nickdesaulniers added a comment.

- add `<` to same comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103928

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Frontend/Wframe-larger-than.c
  clang/test/Frontend/backend-diagnostic.c
  clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
  llvm/include/llvm/IR/Module.h
  llvm/lib/CodeGen/PrologEpilogInserter.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/CodeGen/ARM/warn-stack.ll
  llvm/test/CodeGen/X86/warn-stack.ll
  llvm/test/Linker/warn-stack-frame.ll

Index: llvm/test/Linker/warn-stack-frame.ll
===
--- /dev/null
+++ llvm/test/Linker/warn-stack-frame.ll
@@ -0,0 +1,16 @@
+; RUN: split-file %s %t
+; RUN: llvm-link %t/main.ll %t/match.ll
+; RUN: not llvm-link %t/main.ll %t/mismatch.ll 2>&1 | \
+; RUN:   FileCheck --check-prefix=CHECK-MISMATCH %s
+
+; CHECK-MISMATCH: error: linking module flags 'warn-stack-size': IDs have conflicting values
+
+;--- main.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
+;--- match.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
+;--- mismatch.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 81}
Index: llvm/test/CodeGen/X86/warn-stack.ll
===
--- llvm/test/CodeGen/X86/warn-stack.ll
+++ llvm/test/CodeGen/X86/warn-stack.ll
@@ -1,4 +1,4 @@
-; RUN: llc -mtriple x86_64-apple-macosx10.8.0 -warn-stack-size=80 < %s 2>&1 >/dev/null | FileCheck %s
+; RUN: llc -mtriple x86_64-apple-macosx10.8.0 < %s 2>&1 >/dev/null | FileCheck %s
 ; Check the internal option that warns when the stack size exceeds the
 ; given amount.
 ; 
@@ -22,3 +22,6 @@
 }
 
 declare void @doit(i8*)
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
Index: llvm/test/CodeGen/ARM/warn-stack.ll
===
--- llvm/test/CodeGen/ARM/warn-stack.ll
+++ llvm/test/CodeGen/ARM/warn-stack.ll
@@ -1,4 +1,4 @@
-; RUN: llc -mtriple thumbv7-apple-ios3.0.0 -warn-stack-size=80 < %s 2>&1 >/dev/null | FileCheck %s
+; RUN: llc -mtriple thumbv7-apple-ios3.0.0 < %s 2>&1 >/dev/null | FileCheck %s
 ; Check the internal option that warns when the stack size exceeds the
 ; given amount.
 ; 
@@ -22,3 +22,6 @@
 }
 
 declare void @doit(i8*)
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -732,6 +732,17 @@
   addModuleFlag(ModFlagBehavior::Error, "override-stack-alignment", Align);
 }
 
+unsigned Module::getWarnStackSize() const {
+  Metadata *MD = getModuleFlag("warn-stack-size");
+  if (auto *CI = mdconst::dyn_extract_or_null(MD))
+return CI->getZExtValue();
+  return UINT_MAX;
+}
+
+void Module::setWarnStackSize(unsigned Threshold) {
+  addModuleFlag(ModFlagBehavior::Error, "warn-stack-size", Threshold);
+}
+
 void Module::setSDKVersion(const VersionTuple ) {
   SmallVector Entries;
   Entries.push_back(V.getMajor());
Index: llvm/lib/CodeGen/PrologEpilogInserter.cpp
===
--- llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -138,11 +138,6 @@
 
 char ::PrologEpilogCodeInserterID = PEI::ID;
 
-static cl::opt
-WarnStackSize("warn-stack-size", cl::Hidden, cl::init((unsigned)-1),
-  cl::desc("Warn for stack size bigger than the given"
-   " number"));
-
 INITIALIZE_PASS_BEGIN(PEI, DEBUG_TYPE, "Prologue/Epilogue Insertion", false,
   false)
 INITIALIZE_PASS_DEPENDENCY(MachineLoopInfo)
@@ -278,7 +273,9 @@
   // Warn on stack size when we exceeds the given limit.
   MachineFrameInfo  = MF.getFrameInfo();
   uint64_t StackSize = MFI.getStackSize();
-  if (WarnStackSize.getNumOccurrences() > 0 && WarnStackSize < StackSize) {
+
+  unsigned Threshold = MF.getFunction().getParent()->getWarnStackSize();
+  if (StackSize > Threshold) {
 DiagnosticInfoStackSize DiagStackSize(F, StackSize);
 F.getContext().diagnose(DiagStackSize);
   }
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -913,6 +913,10 @@
   unsigned getOverrideStackAlignment() const;
   void setOverrideStackAlignment(unsigned Align);
 
+  /// Get/set the stack frame size threshold to warn on.
+  unsigned getWarnStackSize() const;
+  void setWarnStackSize(unsigned Threshold);
+
   /// @name Utility functions for querying and setting 

[PATCH] D103928: [IR] make -warn-frame-size into a module attr

2021-06-09 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 351007.
nickdesaulniers added a comment.

- update comment in clang/include/clang/Basic/CodeGenOptions.def


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103928

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Frontend/Wframe-larger-than.c
  clang/test/Frontend/backend-diagnostic.c
  clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
  llvm/include/llvm/IR/Module.h
  llvm/lib/CodeGen/PrologEpilogInserter.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/CodeGen/ARM/warn-stack.ll
  llvm/test/CodeGen/X86/warn-stack.ll
  llvm/test/Linker/warn-stack-frame.ll

Index: llvm/test/Linker/warn-stack-frame.ll
===
--- /dev/null
+++ llvm/test/Linker/warn-stack-frame.ll
@@ -0,0 +1,16 @@
+; RUN: split-file %s %t
+; RUN: llvm-link %t/main.ll %t/match.ll
+; RUN: not llvm-link %t/main.ll %t/mismatch.ll 2>&1 | \
+; RUN:   FileCheck --check-prefix=CHECK-MISMATCH %s
+
+; CHECK-MISMATCH: error: linking module flags 'warn-stack-size': IDs have conflicting values
+
+;--- main.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
+;--- match.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
+;--- mismatch.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 81}
Index: llvm/test/CodeGen/X86/warn-stack.ll
===
--- llvm/test/CodeGen/X86/warn-stack.ll
+++ llvm/test/CodeGen/X86/warn-stack.ll
@@ -1,4 +1,4 @@
-; RUN: llc -mtriple x86_64-apple-macosx10.8.0 -warn-stack-size=80 < %s 2>&1 >/dev/null | FileCheck %s
+; RUN: llc -mtriple x86_64-apple-macosx10.8.0 < %s 2>&1 >/dev/null | FileCheck %s
 ; Check the internal option that warns when the stack size exceeds the
 ; given amount.
 ; 
@@ -22,3 +22,6 @@
 }
 
 declare void @doit(i8*)
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
Index: llvm/test/CodeGen/ARM/warn-stack.ll
===
--- llvm/test/CodeGen/ARM/warn-stack.ll
+++ llvm/test/CodeGen/ARM/warn-stack.ll
@@ -1,4 +1,4 @@
-; RUN: llc -mtriple thumbv7-apple-ios3.0.0 -warn-stack-size=80 < %s 2>&1 >/dev/null | FileCheck %s
+; RUN: llc -mtriple thumbv7-apple-ios3.0.0 < %s 2>&1 >/dev/null | FileCheck %s
 ; Check the internal option that warns when the stack size exceeds the
 ; given amount.
 ; 
@@ -22,3 +22,6 @@
 }
 
 declare void @doit(i8*)
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -732,6 +732,17 @@
   addModuleFlag(ModFlagBehavior::Error, "override-stack-alignment", Align);
 }
 
+unsigned Module::getWarnStackSize() const {
+  Metadata *MD = getModuleFlag("warn-stack-size");
+  if (auto *CI = mdconst::dyn_extract_or_null(MD))
+return CI->getZExtValue();
+  return UINT_MAX;
+}
+
+void Module::setWarnStackSize(unsigned Threshold) {
+  addModuleFlag(ModFlagBehavior::Error, "warn-stack-size", Threshold);
+}
+
 void Module::setSDKVersion(const VersionTuple ) {
   SmallVector Entries;
   Entries.push_back(V.getMajor());
Index: llvm/lib/CodeGen/PrologEpilogInserter.cpp
===
--- llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -138,11 +138,6 @@
 
 char ::PrologEpilogCodeInserterID = PEI::ID;
 
-static cl::opt
-WarnStackSize("warn-stack-size", cl::Hidden, cl::init((unsigned)-1),
-  cl::desc("Warn for stack size bigger than the given"
-   " number"));
-
 INITIALIZE_PASS_BEGIN(PEI, DEBUG_TYPE, "Prologue/Epilogue Insertion", false,
   false)
 INITIALIZE_PASS_DEPENDENCY(MachineLoopInfo)
@@ -278,7 +273,9 @@
   // Warn on stack size when we exceeds the given limit.
   MachineFrameInfo  = MF.getFrameInfo();
   uint64_t StackSize = MFI.getStackSize();
-  if (WarnStackSize.getNumOccurrences() > 0 && WarnStackSize < StackSize) {
+
+  unsigned Threshold = MF.getFunction().getParent()->getWarnStackSize();
+  if (StackSize > Threshold) {
 DiagnosticInfoStackSize DiagStackSize(F, StackSize);
 F.getContext().diagnose(DiagStackSize);
   }
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -913,6 +913,10 @@
   unsigned getOverrideStackAlignment() const;
   void setOverrideStackAlignment(unsigned Align);
 
+  /// Get/set the stack frame size threshold to warn on.
+  unsigned getWarnStackSize() const;
+  void setWarnStackSize(unsigned Threshold);
+
   /// @name 

[PATCH] D103928: [IR] make -warn-frame-size into a module attr

2021-06-09 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 351006.
nickdesaulniers edited the summary of this revision.
nickdesaulniers added a comment.

- introduce -fwarn-stack-size


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103928

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Frontend/Wframe-larger-than.c
  clang/test/Frontend/backend-diagnostic.c
  clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
  llvm/include/llvm/IR/Module.h
  llvm/lib/CodeGen/PrologEpilogInserter.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/CodeGen/ARM/warn-stack.ll
  llvm/test/CodeGen/X86/warn-stack.ll
  llvm/test/Linker/warn-stack-frame.ll

Index: llvm/test/Linker/warn-stack-frame.ll
===
--- /dev/null
+++ llvm/test/Linker/warn-stack-frame.ll
@@ -0,0 +1,16 @@
+; RUN: split-file %s %t
+; RUN: llvm-link %t/main.ll %t/match.ll
+; RUN: not llvm-link %t/main.ll %t/mismatch.ll 2>&1 | \
+; RUN:   FileCheck --check-prefix=CHECK-MISMATCH %s
+
+; CHECK-MISMATCH: error: linking module flags 'warn-stack-size': IDs have conflicting values
+
+;--- main.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
+;--- match.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
+;--- mismatch.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 81}
Index: llvm/test/CodeGen/X86/warn-stack.ll
===
--- llvm/test/CodeGen/X86/warn-stack.ll
+++ llvm/test/CodeGen/X86/warn-stack.ll
@@ -1,4 +1,4 @@
-; RUN: llc -mtriple x86_64-apple-macosx10.8.0 -warn-stack-size=80 < %s 2>&1 >/dev/null | FileCheck %s
+; RUN: llc -mtriple x86_64-apple-macosx10.8.0 < %s 2>&1 >/dev/null | FileCheck %s
 ; Check the internal option that warns when the stack size exceeds the
 ; given amount.
 ; 
@@ -22,3 +22,6 @@
 }
 
 declare void @doit(i8*)
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
Index: llvm/test/CodeGen/ARM/warn-stack.ll
===
--- llvm/test/CodeGen/ARM/warn-stack.ll
+++ llvm/test/CodeGen/ARM/warn-stack.ll
@@ -1,4 +1,4 @@
-; RUN: llc -mtriple thumbv7-apple-ios3.0.0 -warn-stack-size=80 < %s 2>&1 >/dev/null | FileCheck %s
+; RUN: llc -mtriple thumbv7-apple-ios3.0.0 < %s 2>&1 >/dev/null | FileCheck %s
 ; Check the internal option that warns when the stack size exceeds the
 ; given amount.
 ; 
@@ -22,3 +22,6 @@
 }
 
 declare void @doit(i8*)
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -732,6 +732,17 @@
   addModuleFlag(ModFlagBehavior::Error, "override-stack-alignment", Align);
 }
 
+unsigned Module::getWarnStackSize() const {
+  Metadata *MD = getModuleFlag("warn-stack-size");
+  if (auto *CI = mdconst::dyn_extract_or_null(MD))
+return CI->getZExtValue();
+  return UINT_MAX;
+}
+
+void Module::setWarnStackSize(unsigned Threshold) {
+  addModuleFlag(ModFlagBehavior::Error, "warn-stack-size", Threshold);
+}
+
 void Module::setSDKVersion(const VersionTuple ) {
   SmallVector Entries;
   Entries.push_back(V.getMajor());
Index: llvm/lib/CodeGen/PrologEpilogInserter.cpp
===
--- llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -138,11 +138,6 @@
 
 char ::PrologEpilogCodeInserterID = PEI::ID;
 
-static cl::opt
-WarnStackSize("warn-stack-size", cl::Hidden, cl::init((unsigned)-1),
-  cl::desc("Warn for stack size bigger than the given"
-   " number"));
-
 INITIALIZE_PASS_BEGIN(PEI, DEBUG_TYPE, "Prologue/Epilogue Insertion", false,
   false)
 INITIALIZE_PASS_DEPENDENCY(MachineLoopInfo)
@@ -278,7 +273,9 @@
   // Warn on stack size when we exceeds the given limit.
   MachineFrameInfo  = MF.getFrameInfo();
   uint64_t StackSize = MFI.getStackSize();
-  if (WarnStackSize.getNumOccurrences() > 0 && WarnStackSize < StackSize) {
+
+  unsigned Threshold = MF.getFunction().getParent()->getWarnStackSize();
+  if (StackSize > Threshold) {
 DiagnosticInfoStackSize DiagStackSize(F, StackSize);
 F.getContext().diagnose(DiagStackSize);
   }
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -913,6 +913,10 @@
   unsigned getOverrideStackAlignment() const;
   void setOverrideStackAlignment(unsigned Align);
 
+  /// Get/set the stack frame size threshold to warn on.
+  unsigned getWarnStackSize() const;
+  void setWarnStackSize(unsigned Threshold);

[PATCH] D103878: [clang][RISCV][test] Add more tests of the -mabi and -march options

2021-06-09 Thread Ben Shi via Phabricator via cfe-commits
benshi001 added a comment.

In D103878#2809033 , @luismarques 
wrote:

> In D103878#2807118 , @benshi001 
> wrote:
>
>> 1. there is no tests for mabi=ilp32e, and my patch covers that.
>> 2. the tests in riscv-abi.c will show default abi changes for special archs, 
>> especially for the arch with F but without D extension, in the future.
>> 3. the tests in riscv-arch.c will show default arch changes for abi=ilp32, 
>> which is rv32imacfd now, and it is better to be rv32imac. for abi=ilp32f it 
>> is better arch=imacf than current imacfd.
>
> That sounds like a good description to add to the patch summary / commit 
> message!

Done. Thanks.


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

https://reviews.llvm.org/D103878

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


[PATCH] D103958: [WIP] Support MustControl conditional control attribute

2021-06-09 Thread Marco Elver via Phabricator via cfe-commits
melver added a comment.

In D103958#2809145 , @efriedma wrote:

> In D103958#2808966 , @melver wrote:
>
>> In D103958#2808861 , @efriedma 
>> wrote:
>>
>>> I don't like using metadata like this.  Dropping metadata should generally 
>>> preserve the semantics of the code.
>>
>> Anything better for this without introducing new instructions? Would an 
>> argument be reasonable?
>
> If we really want to make it part of the branch, maybe add an intrinsic that 
> can be used with callbr.  Not something we've done before, but the 
> infrastructure should be mostly there.
>
> That said, I'm not sure this is the best approach.  Alternative proposal:
>
> We could add a regular intrinsic.  Just ignore the control flow at the IR 
> level, and come up with a straight-line blob that just does the right thing.  
> I think we'd want to actually perform the load as part of the intrinsic, to 
> avoid worrying about the consume dependency.  So we'd have an intrinsic 
> "__builtin_load_with_control_dependency()". It would lower to something along 
> the lines of  `asm("ldr %0, [%1]; cbnz %0, .+4":"=r"(dest):"r"(x):"memory");` 
> on AArch64.  The differences between the intrinsic and just using the asm I 
> wrote:
>
> 1. We weaken the "memory" clobber to something that more accurately matches 
> what we need.
> 2. We add a compiler transform to check if the branch is redundant, late in 
> the optimization pipeline, and remove it if it is.
>
> I think this produces the code you want, and it should be easier to 
> understand and maintain.

Interesting, but probably doesn't quite work if I understood right -- however, 
perhaps it could solve something related (not part of this work, see below 
[footnote]).

Not every `READ_ONCE()` the kernel has needs to be a 
`load_with_control_dependency()`, which if I read it right, would happen if we 
just do, e.g.:

  #define READ_ONCE __builtin_load_with_control_dependency
  int x;
  void foo(void) { return READ_ONCE(x); }

And they really want to avoid introducing another set of primitives, like 
`READ_ONCE_CTRL()`, because if we did that, I think it'd be reasonable to 
upgrade all `READ_ONCE_CTRL()` to acquires and we're done (suggested by Will 
Deacon in [1]). Yet upgrading all `READ_ONCE()` to acquire is not acceptable in 
general (do note, it's not just AArch64, also POWER and Armv7).  For now, it'd 
be good to avoid this -- in particular, existing code like the following would 
become less clear or less optimal:

  x = READ_ONCE(..);
  y  = READ_ONCE(..);
  ... lots of other code ...
  if (y) { ... do other stuff ... } // <--- no control dependency here
  if (x && y) { WRITE_ONCE(..) } // <-- only want control  dependency here

Which is why the kernel folks probably wouldn't be too happy with more 
primitives as it likely penalizes more than with just marking the branch 
itself. Per [1] new load-primitives are probably a last resort assuming the 
compiler can deliver a nice mechanism to ensure control-dependencies remain 
(this work here).

Thanks.



[1] https://lore.kernel.org/linux-arch/20210607160252.GA7580@willie-the-truck/

[footnote] Re the "memory" clobber, Linus asks for more fine-grained asm 
clobber: 
https://lore.kernel.org/linux-arch/CAHk-=wjwxs5+sozgtaz0bp9nsoa+pymacge4cbdvx3edguc...@mail.gmail.com/
If you see a way to support this, I think it'd help in other places (e.g. 
kernel's one-directional memory barriers).

Tangentially, per this presentation:
https://github.com/ClangBuiltLinux/plumbers-2020-slides/blob/master/LPC_2020_--_Dependency_ordering.pdf
there is another problem, which are address dependencies aka 
`memory_order_consume`. In reality the kernel wants every `READ_ONCE()` be 
something very close to `memory_order_consume`, with the compiler figuring out 
the optimal thing to do. Unfortunately, this is not the reality today. Paul 
McKenney et al. has been exploring this in: 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0750r1.html -- but 
since addr-deps are much less likely to be optimized away, I think the kernel 
will do nothing about it in the near term. ctrl-deps on the other hand are more 
of a worry to the Linux kernel community right now.

I can't summarize this well enough here, so I would strongly recommend: 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0124r6.html#Control%20Dependencies


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103958

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


[PATCH] D103995: [OpenMP] Add type to firstprivate symbol for const firstprivate values

2021-06-09 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, ABataev.
Herald added subscribers: guansong, yaxunl.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Clang will create a global value put in constant memory if an aggregate value
is declared firstprivate in the target device. The symbol name only uses the
name of the firstprivate variable, so symbol name conflicts will occur if the
variable is allowed to have different types through templates. An example of
this behvaiour is shown in https://godbolt.org/z/EsMjYh47n. This patch adds the
mangled type name to the symbol to avoid such naming conflicts. This fixes
https://bugs.llvm.org/show_bug.cgi?id=50642.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103995

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp


Index: clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
===
--- clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
+++ clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
@@ -13,11 +13,14 @@
   ty Y;
 };
 
-// TCHECK-DAG:  [[TT:%.+]] = type { i64, i8 }
 // TCHECK-DAG:  [[TTII:%.+]] = type { i32, i32 }
+// TCHECK-DAG:  [[TTIC:%.+]] = type { i8, i8 }
+// TCHECK-DAG:  [[TT:%.+]] = type { i64, i8 }
 // TCHECK-DAG:  [[S1:%.+]] = type { double }
 
-// TCHECK: @__omp_offloading_firstprivate__{{.+}}_e_l27 = internal 
addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_e_l30 = internal 
addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_ZTSK2TTIiiE_t_l143 = 
internal addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_ZTSK2TTIccE_t_l143 = 
internal addrspace(4) global [[TTIC]] zeroinitializer
 int foo(int n, double *ptr) {
   int a = 0;
   short aa = 0;
@@ -136,6 +139,12 @@
   return a;
 }
 
+template 
+void fconst(const tx t) {
+#pragma omp target firstprivate(t)
+  { }
+}
+
 // TCHECK: define {{.*}}void @__omp_offloading_{{.+}}(i{{[0-9]+}}{{.*}} 
[[A_IN:%.+]], i{{[0-9]+}}{{.*}} [[A3_IN:%.+]], [10 x i{{[0-9]+}}]*{{.+}} 
[[B_IN:%.+]])
 // TCHECK:  [[A_ADDR:%.+]] = alloca i{{[0-9]+}},
 // TCHECK:  [[A3_ADDR:%.+]] = alloca i{{[0-9]+}},
@@ -199,6 +208,9 @@
   a += fstatic(n);
   a += ftemplate(n);
 
+  fconst(TT{0, 0});
+  fconst(TT{0, 0});
+
   return a;
 }
 
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -10593,7 +10593,9 @@
  FileID, Line);
 llvm::raw_svector_ostream OS(Buffer);
 OS << "__omp_offloading_firstprivate_" << llvm::format("_%x", DeviceID)
-   << llvm::format("_%x_", FileID) << VD->getName() << "_l" << Line;
+   << llvm::format("_%x_", FileID);
+CGM.getCXXABI().getMangleContext().mangleTypeName(VD->getType(), OS);
+OS << "_" << VD->getName() << "_l" << Line;
 VarName = OS.str();
   }
   Linkage = llvm::GlobalValue::InternalLinkage;


Index: clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
===
--- clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
+++ clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
@@ -13,11 +13,14 @@
   ty Y;
 };
 
-// TCHECK-DAG:  [[TT:%.+]] = type { i64, i8 }
 // TCHECK-DAG:  [[TTII:%.+]] = type { i32, i32 }
+// TCHECK-DAG:  [[TTIC:%.+]] = type { i8, i8 }
+// TCHECK-DAG:  [[TT:%.+]] = type { i64, i8 }
 // TCHECK-DAG:  [[S1:%.+]] = type { double }
 
-// TCHECK: @__omp_offloading_firstprivate__{{.+}}_e_l27 = internal addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_e_l30 = internal addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_ZTSK2TTIiiE_t_l143 = internal addrspace(4) global [[TTII]] zeroinitializer
+// TCHECK: @__omp_offloading_firstprivate__{{.+}}_ZTSK2TTIccE_t_l143 = internal addrspace(4) global [[TTIC]] zeroinitializer
 int foo(int n, double *ptr) {
   int a = 0;
   short aa = 0;
@@ -136,6 +139,12 @@
   return a;
 }
 
+template 
+void fconst(const tx t) {
+#pragma omp target firstprivate(t)
+  { }
+}
+
 // TCHECK: define {{.*}}void @__omp_offloading_{{.+}}(i{{[0-9]+}}{{.*}} [[A_IN:%.+]], i{{[0-9]+}}{{.*}} [[A3_IN:%.+]], [10 x i{{[0-9]+}}]*{{.+}} [[B_IN:%.+]])
 // TCHECK:  [[A_ADDR:%.+]] = alloca i{{[0-9]+}},
 // TCHECK:  [[A3_ADDR:%.+]] = alloca i{{[0-9]+}},
@@ -199,6 +208,9 @@
   a += fstatic(n);
   a += ftemplate(n);
 
+  fconst(TT{0, 0});
+  fconst(TT{0, 0});
+
   return a;
 }
 
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ 

[PATCH] D103928: [IR] make -warn-frame-size into a module attr

2021-06-09 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2575-2581
 // These "special" warning flags are effectively processed as f_Group flags by 
the driver:
 // Just silence warnings about -Wlarger-than for now.
 def Wlarger_than_EQ : Joined<["-"], "Wlarger-than=">, 
Group;
 def Wlarger_than_ : Joined<["-"], "Wlarger-than-">, Alias;
-def Wframe_larger_than_EQ : Joined<["-"], "Wframe-larger-than=">, 
Group, Flags<[NoXarchOption]>;
+def Wframe_larger_than_EQ : Joined<["-"], "Wframe-larger-than=">,
+  Group, Flags<[NoXarchOption,CC1Option]>,
+  MarshallingInfoInt, "UINT_MAX">;

nickdesaulniers wrote:
> @rsmith I wasn't super sure about this hunk of the diff.  Should I not be 
> reusing the same flag between the Frontend and the Driver? (The Driver comes 
> after the Frontend, IIUC? Is that right?)
(The driver comes before the frontend: the driver is the user-facing 
GCC-compatible `clang` binary, and the frontend is the `-cc1` interface.)

What we do in other cases is to translate the driver-level (user-facing) 
warning flag into multiple frontend flags:

* The driver-level flag `-Wwrite-strings` (in C) gets mapped into the 
frontend-level flags `-Wwrite-strings -fconst-strings`, where `-Wwrite-strings` 
only controls whether the warning is shown, and `-fconst-strings` only controls 
whether string literals have type `char[N]` or `const char[N]`.
* The driver-level flag `-Wdeprecated` (in C++) gets mapped into the 
frontend-level flags `-Wdeprecated -fdeprecated-macro`, where `-Wdeprecated` 
only controls whether the warning is shown, and `-fdeprecated-macro` only 
controls whether `__DEPRECATED` is implicitly defined by the preprocessor.

So I think the expected strategy here would probably be that the driver-level 
flag `-Wframe-larger-than=N` would get mapped into the frontend-level flags 
`-Wframe-larger-than -fsomething=N`, where `-Wframe-larger-than` only controls 
whether the warning is shown, and `-fsomething=N` sets the level of stack usage 
at which the backend triggers the warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103928

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


[clang] cf11d95 - Fix to Windows temp file change.

2021-06-09 Thread Amy Huang via cfe-commits

Author: Amy Huang
Date: 2021-06-09T16:15:16-07:00
New Revision: cf11d9585afd5c43031a9fb622c8c31b4bef

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

LOG: Fix to Windows temp file change.

Original change passed wrong parameters to the raw_fd_ostream ctor.
Fixes a bug in https://reviews.llvm.org/D102736.

Added: 


Modified: 
clang/lib/Frontend/CompilerInstance.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index a70fe95800e1..78c7d84bbef4 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -853,9 +853,7 @@ CompilerInstance::createOutputFileImpl(StringRef 
OutputPath, bool Binary,
   consumeError(std::move(E));
 } else {
   Temp = std::move(ExpectedFile.get());
-  OS.reset(new llvm::raw_fd_ostream(Temp->FD, /*shouldClose=*/false,
-Binary ? llvm::sys::fs::OF_None
-   : llvm::sys::fs::OF_Text));
+  OS.reset(new llvm::raw_fd_ostream(Temp->FD, /*shouldClose=*/false));
   OSFile = Temp->TmpName;
 }
 // If we failed to create the temporary, fallback to writing to the file



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


[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

2021-06-09 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment.

In D99696#2808592 , @Quuxplusone wrote:

> @aaronpuchert what's your take on D99696  at 
> this point; is it good to go or still unresolved issues?

Overall this looks sensible to me, and I don't really have anything to add. 
Parts of the code seem a bit complex, but that just seems to reflect the 
complexity of the standard(s).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

___
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.

2021-06-09 Thread Amy Huang via Phabricator via cfe-commits
akhuang added inline comments.



Comment at: clang/lib/Frontend/CompilerInstance.cpp:857-858
+  OS.reset(new llvm::raw_fd_ostream(Temp->FD, /*shouldClose=*/false,
+Binary ? llvm::sys::fs::OF_None
+   : llvm::sys::fs::OF_Text));
+  OSFile = std::string(TempPath.str());

Quuxplusone wrote:
> amccarth wrote:
> > rnk wrote:
> > > I think the bug is here: the third parameter is `bool unbuffered`, not 
> > > file flags, so we are opening the file for unbuffered writing, and that 
> > > is really slow.
> > Yowza!  In addition to a fix, we need some memes.
> > 
> > * Why do we even have that lever?
> > * This constructor has too many overloads.  Please remove three.
> > * [Facepalm] `bool` parameters.  [Double facepalm]  Negative `bool` 
> > parameter names that default to `false`.
> > In addition to a fix, we need some memes.
> 
> https://quuxplusone.github.io/blog/2020/04/18/default-function-arguments-are-the-devil/
>  ;)
> I count 8 different signatures for `raw_fd_ostream`'s constructor, 3 of which 
> come from the offending default-argument'ed overload. And none of them are 
> `explicit`, which means e.g. `{42, false}` is a valid argument for a function 
> taking `const raw_fd_ostream&`.
> 
> Unrelated to argument lists, but I would add "using raw `new` instead of 
> `std::make_unique`" to this line's lengthy list of sins.
oh whoops, thanks for the catch - for now I guess I'll just remove the extra 
parameter


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] D102736: Fix tmp files being left on Windows builds.

2021-06-09 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: clang/lib/Frontend/CompilerInstance.cpp:857-858
+  OS.reset(new llvm::raw_fd_ostream(Temp->FD, /*shouldClose=*/false,
+Binary ? llvm::sys::fs::OF_None
+   : llvm::sys::fs::OF_Text));
+  OSFile = std::string(TempPath.str());

amccarth wrote:
> rnk wrote:
> > I think the bug is here: the third parameter is `bool unbuffered`, not file 
> > flags, so we are opening the file for unbuffered writing, and that is 
> > really slow.
> Yowza!  In addition to a fix, we need some memes.
> 
> * Why do we even have that lever?
> * This constructor has too many overloads.  Please remove three.
> * [Facepalm] `bool` parameters.  [Double facepalm]  Negative `bool` parameter 
> names that default to `false`.
> In addition to a fix, we need some memes.

https://quuxplusone.github.io/blog/2020/04/18/default-function-arguments-are-the-devil/
 ;)
I count 8 different signatures for `raw_fd_ostream`'s constructor, 3 of which 
come from the offending default-argument'ed overload. And none of them are 
`explicit`, which means e.g. `{42, false}` is a valid argument for a function 
taking `const raw_fd_ostream&`.

Unrelated to argument lists, but I would add "using raw `new` instead of 
`std::make_unique`" to this line's lengthy list of sins.


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] D102736: Fix tmp files being left on Windows builds.

2021-06-09 Thread Adrian McCarthy via Phabricator via cfe-commits
amccarth added a comment.

Nice catch Reid.




Comment at: clang/lib/Frontend/CompilerInstance.cpp:857-858
+  OS.reset(new llvm::raw_fd_ostream(Temp->FD, /*shouldClose=*/false,
+Binary ? llvm::sys::fs::OF_None
+   : llvm::sys::fs::OF_Text));
+  OSFile = std::string(TempPath.str());

rnk wrote:
> I think the bug is here: the third parameter is `bool unbuffered`, not file 
> flags, so we are opening the file for unbuffered writing, and that is really 
> slow.
Yowza!  In addition to a fix, we need some memes.

* Why do we even have that lever?
* This constructor has too many overloads.  Please remove three.
* [Facepalm] `bool` parameters.  [Double facepalm]  Negative `bool` parameter 
names that default to `false`.


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] D102782: Add support for Warning Flag "-Wstack-usage="

2021-06-09 Thread Ryan Santhirarajan via Phabricator via cfe-commits
rsanthir.quic added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:79-80
 
+- ``-Wstack-usage=`` warn if stack usage of user functions might
+  exceed .
+

Quuxplusone wrote:
> Does this mean:
> - Warn if the size of any single function's stack frame (including 
> temporaries and local variables, but not parameters or return addresses) 
> exceeds .
> - Warn if the size of any single function's stack frame (including parameters 
> and return addresses) exceeds .
> - Warn if the total stack usage of the longest visible call chain in this 
> translation unit might exceed .
> ?
> 
Looking at how the value for this flag is obtained, which is an alias of 
"-Wframe-larger-than", I see that it is all objects placed on the stack by each 
function plus any space reserved for arguments of callsites in the function. 

There is no precise definition of what is included as it depends on the target 
as well as what is stored on the stack or not.

I guess in simplest terms, for each function this will report how much stack 
space is used by that function.




Comment at: clang/test/Frontend/backend-stack-usage-diagnostic.c:19-23
+// WARN: warning: stack frame size of {{[0-9]+}} bytes in function 
'test_square'
+// IGNORE-NOT:  stack frame size of {{[0-9]+}} bytes in function 'test_square'
+int test_square(int num) {
+  return num * num;
+}

Quuxplusone wrote:
> This function has no "stack frame" in the usual sense of the word, because 
> it's just
> ```
> movl%edi, %eax
> imull   %edi, %eax
> ret
> ```
> So am I correct to infer that the `-Wstack-usage=` option includes the size 
> of the return address in its computations? So the stack usage of this 
> function would be computed as "8 bytes" because the `callq` instruction 
> pushes 8 bytes on the stack?
> This seems eminently reasonable to me, but non-obvious, and should get some 
> user-facing documentation.
When a function stores the return address on the stack, that will be included 
in that functions stack usage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102782

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


[PATCH] D103958: [WIP] Support MustControl conditional control attribute

2021-06-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

In D103958#2808966 , @melver wrote:

> In D103958#2808861 , @efriedma 
> wrote:
>
>> I don't like using metadata like this.  Dropping metadata should generally 
>> preserve the semantics of the code.
>
> Anything better for this without introducing new instructions? Would an 
> argument be reasonable?

If we really want to make it part of the branch, maybe add an intrinsic that 
can be used with callbr.  Not something we've done before, but the 
infrastructure should be mostly there.

That said, I'm not sure this is the best approach.  Alternative proposal:

We could add a regular intrinsic.  Just ignore the control flow at the IR 
level, and come up with a straight-line blob that just does the right thing.  I 
think we'd want to actually perform the load as part of the intrinsic, to avoid 
worrying about the consume dependency.  So we'd have an intrinsic 
"__builtin_load_with_control_dependency()". It would lower to something along 
the lines of  `asm("ldr %0, [%1]; cbnz %0, .+4":"=r"(dest):"r"(x):"memory");` 
on AArch64.  The differences between the intrinsic and just using the asm I 
wrote:

1. We weaken the "memory" clobber to something that more accurately matches 
what we need.
2. We add a compiler transform to check if the branch is redundant, late in the 
optimization pipeline, and remove it if it is.

I think this produces the code you want, and it should be easier to understand 
and maintain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103958

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


[PATCH] D103987: Start tracking Clang's C implementation status

2021-06-09 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

Sounds like a great plan to me.

I might suggest adding some text about the page being incomplete, so that 
people don't wonder if the blank sections for pre-C2x are a bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103987

___
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.

2021-06-09 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: clang/lib/Frontend/CompilerInstance.cpp:857-858
+  OS.reset(new llvm::raw_fd_ostream(Temp->FD, /*shouldClose=*/false,
+Binary ? llvm::sys::fs::OF_None
+   : llvm::sys::fs::OF_Text));
+  OSFile = std::string(TempPath.str());

I think the bug is here: the third parameter is `bool unbuffered`, not file 
flags, so we are opening the file for unbuffered writing, and that is really 
slow.


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] D103495: [static initializers] Emit global_ctors and global_dtors in reverse order when init_array is not used.

2021-06-09 Thread Wolfgang Pieb via Phabricator via cfe-commits
wolfgangp added a comment.

In D103495#2808247 , @rnk wrote:

> So, a question for @wolfgangp and @probinson , do we need to make adjustments 
> to the LTO library, or is this OK as is?

We never guaranteed our licensees any particular initialization order (between 
modules), so after checking with my team members I think this is OK as is.


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

https://reviews.llvm.org/D103495

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


[PATCH] D100713: [clang] NFC: refactor usage of getDecltypeForParenthesizedExpr

2021-06-09 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov updated this revision to Diff 350998.
mizvekov added a comment.

rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100713

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprObjC.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp

Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -69,29 +69,7 @@
 
 QualType CallEvent::getResultType() const {
   ASTContext  = getState()->getStateManager().getContext();
-  const Expr *E = getOriginExpr();
-  if (!E)
-return Ctx.VoidTy;
-  assert(E);
-
-  QualType ResultTy = E->getType();
-
-  // A function that returns a reference to 'int' will have a result type
-  // of simply 'int'. Check the origin expr's value kind to recover the
-  // proper type.
-  switch (E->getValueKind()) {
-  case VK_LValue:
-ResultTy = Ctx.getLValueReferenceType(ResultTy);
-break;
-  case VK_XValue:
-ResultTy = Ctx.getRValueReferenceType(ResultTy);
-break;
-  case VK_PRValue:
-// No adjustment is necessary.
-break;
-  }
-
-  return ResultTy;
+  return Ctx.getDecltypeForParenthesizedExpr(getOriginExpr());
 }
 
 static bool isCallback(QualType T) {
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8869,29 +8869,6 @@
   return Context.getTypeOfExprType(E);
 }
 
-/// getDecltypeForParenthesizedExpr - Given an expr, will return the type for
-/// that expression, as in [dcl.type.simple]p4 but without taking id-expressions
-/// and class member access into account.
-QualType Sema::getDecltypeForParenthesizedExpr(Expr *E) {
-  // C++11 [dcl.type.simple]p4:
-  //   [...]
-  QualType T = E->getType();
-  switch (E->getValueKind()) {
-  // - otherwise, if e is an xvalue, decltype(e) is T&&, where T is the
-  //   type of e;
-  case VK_XValue:
-return Context.getRValueReferenceType(T);
-  // - otherwise, if e is an lvalue, decltype(e) is T&, where T is the
-  //   type of e;
-  case VK_LValue:
-return Context.getLValueReferenceType(T);
-  //  - otherwise, decltype(e) is the type of e.
-  case VK_PRValue:
-return T;
-  }
-  llvm_unreachable("Unknown value kind");
-}
-
 /// getDecltypeForExpr - Given an expr, will return the decltype for
 /// that expression, according to the rules in C++11
 /// [dcl.type.simple]p4 and C++11 [expr.lambda.prim]p18.
@@ -8956,7 +8933,7 @@
 }
   }
 
-  return S.getDecltypeForParenthesizedExpr(E);
+  return S.Context.getDecltypeForParenthesizedExpr(E);
 }
 
 QualType Sema::BuildDecltypeType(Expr *E, SourceLocation Loc,
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5866,10 +5866,8 @@
   //   -- If E2 is an xvalue: E1 can be converted to match E2 if E1 can be
   //  implicitly converted to the type "rvalue reference to R2", subject to
   //  the constraint that the reference must bind directly.
-  if (To->isLValue() || To->isXValue()) {
-QualType T = To->isLValue() ? Self.Context.getLValueReferenceType(ToType)
-: Self.Context.getRValueReferenceType(ToType);
-
+  if (To->isGLValue()) {
+QualType T = Self.Context.getDecltypeForParenthesizedExpr(To);
 InitializedEntity Entity = InitializedEntity::InitializeTemporary(T);
 
 InitializationSequence InitSeq(Self, Entity, Kind, From);
@@ -8713,7 +8711,7 @@
 TemplateParameterList *TPL =
 ReturnTypeRequirement.getTypeConstraintTemplateParameterList();
 QualType MatchedType =
-getDecltypeForParenthesizedExpr(E).getCanonicalType();
+Context.getDecltypeForParenthesizedExpr(E).getCanonicalType();
 llvm::SmallVector Args;
 Args.push_back(TemplateArgument(MatchedType));
 TemplateArgumentList TAL(TemplateArgumentList::OnStack, Args);
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19244,14 +19244,8 @@
 if (ParamTypes.empty() && Proto->isVariadic()) { // the special case
   ArgTypes.reserve(E->getNumArgs());
   for (unsigned i = 0, e = E->getNumArgs(); i != e; ++i) {
-Expr *Arg = E->getArg(i);
-QualType ArgType = Arg->getType();
-if (E->isLValue()) {
-  ArgType = S.Context.getLValueReferenceType(ArgType);
-} else if (E->isXValue()) {
-  ArgType = S.Context.getRValueReferenceType(ArgType);
-

[PATCH] D103021: [clang-tidy] performance-unnecessary-copy-initialization: Search whole function body for variable initializations.

2021-06-09 Thread Felix Berger via Phabricator via cfe-commits
flx added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:98-101
   auto Matches =
   match(findAll(declStmt(has(varDecl(equalsNode(
 .bind("declStmt")),
+Body, Context);

ymandel wrote:
> Consider inspecting the `DeclContext`s instead, which should be much more 
> efficient than searching the entire block.  Pass the `FunctionDecl` as an 
> argument instead of `Body`, since it is a `DeclContext`.  e.g. `const 
> DeclContext `
> 
> Then, either
> 1. Call `Fun.containsDecl(InitializingVar)`, or
> 2. Search through the contexts yourself; something like:
> ```
> DeclContext* DC = InitializingVar->getDeclContext(); 
> while (DC != nullptr && DC != )
>   DC = DC->getLexicalParent();
> if (DC == nullptr)
>   // The reference or pointer is not initialized anywhere witin the function. 
> We
>   // assume its pointee is not modified then.
>   return true;
> ```
Are #1 and #2 equivalent? From the implementation and comment I cannot tell 
whether #1 would cover cases where the variable is not declared directly in the 
function, but in child block:

```
void Fun() {
 {
   var i;
   {
 i.usedHere();
   }  
 } 
}
```

I'm also reading this as an optimization to more quickly determine whether we 
can stop here. We still need to find the matches for the next steps, but I  
think I could then limit matching to the DeclContext I found here. Is this 
correct? For this I would actually need the DeclContext result from #2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103021

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


[PATCH] D103021: [clang-tidy] performance-unnecessary-copy-initialization: Search whole function body for variable initializations.

2021-06-09 Thread Felix Berger via Phabricator via cfe-commits
flx updated this revision to Diff 350994.
flx marked 2 inline comments as done.
flx added a comment.

Addressed first round of comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103021

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -1,9 +1,19 @@
 // RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
 
+template 
+struct Iterator {
+  void operator++();
+  const T *() const;
+  bool operator!=(const Iterator &) const;
+  typedef const T _reference;
+};
+
 struct ExpensiveToCopyType {
   ExpensiveToCopyType();
   virtual ~ExpensiveToCopyType();
   const ExpensiveToCopyType () const;
+  Iterator begin() const;
+  Iterator end() const;
   void nonConstMethod();
   bool constMethod() const;
 };
@@ -508,3 +518,41 @@
   // CHECK-FIXES: const auto& UnnecessaryCopy = Ref.reference();
   Orig.constMethod();
 }
+
+void negativeloopedOverObjectIsModified() {
+  ExpensiveToCopyType Orig;
+  for (const auto  : Orig) {
+const auto Copy = Element;
+Orig.nonConstMethod();
+Copy.constMethod();
+  }
+
+  auto Lambda = []() {
+ExpensiveToCopyType Orig;
+for (const auto  : Orig) {
+  const auto Copy = Element;
+  Orig.nonConstMethod();
+  Copy.constMethod();
+}
+  };
+}
+
+void negativeReferenceIsInitializedOutsideOfBlock() {
+  ExpensiveToCopyType Orig;
+  const auto  = Orig;
+  if (1 != 2 * 3) {
+const auto C2 = E2;
+Orig.nonConstMethod();
+C2.constMethod();
+  }
+
+  auto Lambda = []() {
+ExpensiveToCopyType Orig;
+const auto  = Orig;
+if (1 != 2 * 3) {
+  const auto C2 = E2;
+  Orig.nonConstMethod();
+  C2.constMethod();
+}
+  };
+}
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -35,11 +35,12 @@
 
 private:
   void handleCopyFromMethodReturn(const VarDecl , const Stmt ,
-  bool IssueFix, const VarDecl *ObjectArg,
+  const Stmt , bool IssueFix,
+  const VarDecl *ObjectArg,
   ASTContext );
   void handleCopyFromLocalVar(const VarDecl , const VarDecl ,
-  const Stmt , bool IssueFix,
-  ASTContext );
+  const Stmt , const Stmt ,
+  bool IssueFix, ASTContext );
   const std::vector AllowedTypes;
 };
 
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -82,6 +82,7 @@
 // object arg or variable that is referenced is immutable as well.
 static bool isInitializingVariableImmutable(const VarDecl ,
 const Stmt ,
+const Stmt ,
 ASTContext ) {
   if (!isOnlyUsedAsConst(InitializingVar, BlockStmt, Context))
 return false;
@@ -92,12 +93,14 @@
   if (!isa(T))
 return true;
 
+  // Search the whole function body, not just the current block statement, for
+  // decl statements of the initialization variable.
   auto Matches =
   match(findAll(declStmt(has(varDecl(equalsNode(
 .bind("declStmt")),
-BlockStmt, Context);
-  // The reference or pointer is not initialized in the BlockStmt. We assume
-  // its pointee is not modified then.
+Body, Context);
+  // The reference or pointer is not initialized anywhere witin the function. We
+  // assume its pointee is not modified then.
   if (Matches.empty())
 return true;
 
@@ -110,10 +113,10 @@
 return true;
   // Check that the object argument is immutable as well.
   if (const auto *OrigVar = selectFirst(ObjectArgId, Matches))
-return isInitializingVariableImmutable(*OrigVar, BlockStmt, Context);
+return isInitializingVariableImmutable(*OrigVar, BlockStmt, Body, Context);
   // Check that the old 

[PATCH] D103878: [clang][RISCV][test] Add more tests of the -mabi and -march options

2021-06-09 Thread Luís Marques via Phabricator via cfe-commits
luismarques added a comment.

In D103878#2807118 , @benshi001 wrote:

> 1. there is no tests for mabi=ilp32e, and my patch covers that.
> 2. the tests in riscv-abi.c will show default abi changes for special archs, 
> especially for the arch with F but without D extension, in the future.
> 3. the tests in riscv-arch.c will show default arch changes for abi=ilp32, 
> which is rv32imacfd now, and it is better to be rv32imac. for abi=ilp32f it 
> is better arch=imacf than current imacfd.

That sounds like a good description to add to the patch summary / commit 
message!


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

https://reviews.llvm.org/D103878

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


[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

2021-06-09 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment.

In D99696#2808592 , @Quuxplusone wrote:

> @mizvekov, my understanding is:
>
> - D99696  was temporarily blocked on D103720 
> , but now D103720 
>  is landed and D99696 
>  is unblocked
> - D99696  is a codegen improvement in all 
> language modes, and is also blocking D99005 
> - D99005  is blocking our getting experience 
> with p2266  for C++23
>
> IMHO we should just plow ahead here. @aaronpuchert what's your take on D99696 
>  at this point; is it good to go or still 
> unresolved issues?

No, actually, D99696  (This DR) was not 
blocked on any other DRs, I just put it as parent here for my convenience, 
since I had them all stacked on my workstation to make my life easier.
Sorry if that caused confusion and anyone thought it was blocked

D99005  (The P2266 
 implementation) is blocked just on the present 
DR, but that was because there are common changes and I decided to do them in 
this order since it was not clear when P2266  
impl could be merged.

D103720  was a dependency for D100733 
 proposed by @rsmith, which is a dependency 
of D10  (This is the one that implements 
the 'almost xvalue' thing that Jason Merril was pushing on the reflector, in 
order to replace the two-phase overload resolution).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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


[PATCH] D103958: [WIP] Support MustControl conditional control attribute

2021-06-09 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added subscribers: nhaehnle, rjmccall, arsenm.
jdoerfert added a comment.

This starts to sound an awful lot like `convergent` to me, basically: Do not 
change the control conditions of this call.
Still unsure, maybe you can add a LangRef draft so we know what you try to do, 
or a nice example what you don't want to happen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103958

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


[PATCH] D103958: [WIP] Support MustControl conditional control attribute

2021-06-09 Thread Marco Elver via Phabricator via cfe-commits
melver added a comment.

In D103958#2808967 , @jdoerfert wrote:

>> Please bear with me, I'm updating examples and documentation. I didn't think 
>> anybody would look at this while [WIP]. :-)
>
> People try to help so you have early design feedback ;)

Thank you for that. The LKML discussion got a little heated, so I worry 
slightly that underpresenting the issue will cause a bad first impression.

But while we're here:

There is the consideration to make this a __builtin and not an attribute.

AFAIK a __builtin suffers from the usual problem that the information cannot be 
propagated between TUs:

  file1.c:
bool foo(void) { return __builtin_mustcontrol(READ_ONCE(...)); }
  
  file2.c:
void bar(void) { if (foo()) { WRITE_ONCE(...); } }

Or is a language builtin that gives you an error when used in the wrong context 
acceptable? It seems a little odd because I'm unaware of other builtins that do 
that.

GCC devs expressed that GNU attribute syntax may be abused: 
https://lkml.kernel.org/r/20210609171419.gi18...@gate.crashing.org

The attribute is simpler, and hypothetically, if it were to become part of the 
language std we'd have to use [[...]] syntax anyway, so the GNU attribute 
problem seems somewhat artificial to me.

  [[mustcontrol]] if (READ_ONCE(...)) { ... }
  [[mustcontrol]] while (...) { }

Preferences?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103958

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


[PATCH] D103958: [WIP] Support MustControl conditional control attribute

2021-06-09 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

> Please bear with me, I'm updating examples and documentation. I didn't think 
> anybody would look at this while [WIP]. :-)

People try to help so you have early design feedback ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103958

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


[PATCH] D103958: [WIP] Support MustControl conditional control attribute

2021-06-09 Thread Marco Elver via Phabricator via cfe-commits
melver added a comment.

In D103958#2808861 , @efriedma wrote:

> I don't like using metadata like this.  Dropping metadata should generally 
> preserve the semantics of the code.

Anything better for this without introducing new instructions? Would an 
argument be reasonable?

>> without resorting to inline assembly (which often results in poor codegen).
>
> Could you give an example of the resulting assembly with mustcontrol vs. this 
> patch?

For one of the pathological cases:

  int x, y, z;  


   



 
  int main(int argc, char *argv[]) {


   
  z = 42;   


 



 
volatile_if (READ_ONCE(x)) {


   
WRITE_ONCE(y, z);   


 
  } else {  


 
WRITE_ONCE(y, z);   


 
  } 


 



 
  return 0; 


 
  }

Doing nothing:

  define dso_local i32 @main(i32 %argc, i8** nocapture readnone %argv) 
local_unnamed_addr #0 {
  entry:
store i32 42, i32* @z, align 4, !tbaa !3
%0 = load volatile i32, i32* @x, align 4, !tbaa !3
store volatile i32 42, i32* @y, align 4, !tbaa !3
ret i32 0
  }

No branch here.

Their latest proposal using compiler barriers (and not asmgoto):

  define dso_local i32 @main(i32 %0, i8** nocapture readnone %1) 
local_unnamed_addr #0 {
store i32 42, i32* @z, align 4, !tbaa !2
%3 = load volatile i32, i32* @x, align 4, !tbaa !2
%4 = icmp eq i32 %3, 0
br i1 %4, label %7, label %5
  
  5:; preds = %2
tail call void asm sideeffect "", 
"i,~{memory},~{dirflag},~{fpsr},~{flags}"(i32 0) #1, !srcloc !6 
   
%6 = load i32, i32* @z, align 4, !tbaa !2
br label %7
  
  7:; preds = %2, %5
%8 = phi i32 [ %6, %5 ], [ 42, %2 ]
store volatile i32 %8, i32* @y, align 4, !tbaa !2
ret i32 0
  }

You can see the unnecessary load to z.

With mustcontrol:

  define dso_local i32 @main(i32 %argc, i8** nocapture readnone %argv) 
local_unnamed_addr #0 {
  entry:
store i32 42, i32* @z, align 4, !tbaa !3
%0 = load volatile i32, i32* @x, align 4, !tbaa !3
 

[PATCH] D103018: [clang-tidy] performance-unnecessary-copy-initialization: Look at the canonical type when checking for aliases.

2021-06-09 Thread Felix Berger via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGefa4dbc32ca9: [clang-tidy] 
performance-unnecessary-copy-initialization: Look at the canonical… (authored 
by flx).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103018

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -4,6 +4,7 @@
   ExpensiveToCopyType();
   virtual ~ExpensiveToCopyType();
   const ExpensiveToCopyType () const;
+  const ExpensiveToCopyType *pointer() const;
   void nonConstMethod();
   bool constMethod() const;
 };
@@ -548,6 +549,25 @@
   Orig.nonConstMethod();
 }
 
+void negativeAliasNonCanonicalPointerType() {
+  ExpensiveToCopyType Orig;
+  // The use of auto here hides that the type is a pointer type. The check 
needs
+  // to look at the canonical type to detect the aliasing through this pointer.
+  const auto Pointer = Orig.pointer();
+  const auto NecessaryCopy = Pointer->reference();
+  Orig.nonConstMethod();
+}
+
+void negativeAliasTypedefedType() {
+  typedef const ExpensiveToCopyType 
+  ExpensiveToCopyType Orig;
+  // The typedef hides the fact that this is a reference type. The check needs
+  // to look at the canonical type to detect the aliasing.
+  ReferenceType Ref = Orig.reference();
+  const auto NecessaryCopy = Ref.reference();
+  Orig.nonConstMethod();
+}
+
 void positiveCopiedFromGetterOfReferenceToConstVar() {
   ExpensiveToCopyType Orig;
   const auto  = Orig.reference();
Index: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -100,7 +100,7 @@
   if (!isOnlyUsedAsConst(InitializingVar, BlockStmt, Context))
 return false;
 
-  QualType T = InitializingVar.getType();
+  QualType T = InitializingVar.getType().getCanonicalType();
   // The variable is a value type and we know it is only used as const. Safe
   // to reference it and avoid the copy.
   if (!isa(T))


Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -4,6 +4,7 @@
   ExpensiveToCopyType();
   virtual ~ExpensiveToCopyType();
   const ExpensiveToCopyType () const;
+  const ExpensiveToCopyType *pointer() const;
   void nonConstMethod();
   bool constMethod() const;
 };
@@ -548,6 +549,25 @@
   Orig.nonConstMethod();
 }
 
+void negativeAliasNonCanonicalPointerType() {
+  ExpensiveToCopyType Orig;
+  // The use of auto here hides that the type is a pointer type. The check needs
+  // to look at the canonical type to detect the aliasing through this pointer.
+  const auto Pointer = Orig.pointer();
+  const auto NecessaryCopy = Pointer->reference();
+  Orig.nonConstMethod();
+}
+
+void negativeAliasTypedefedType() {
+  typedef const ExpensiveToCopyType 
+  ExpensiveToCopyType Orig;
+  // The typedef hides the fact that this is a reference type. The check needs
+  // to look at the canonical type to detect the aliasing.
+  ReferenceType Ref = Orig.reference();
+  const auto NecessaryCopy = Ref.reference();
+  Orig.nonConstMethod();
+}
+
 void positiveCopiedFromGetterOfReferenceToConstVar() {
   ExpensiveToCopyType Orig;
   const auto  = Orig.reference();
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -100,7 +100,7 @@
   if (!isOnlyUsedAsConst(InitializingVar, BlockStmt, Context))
 return false;
 
-  QualType T = InitializingVar.getType();
+  QualType T = InitializingVar.getType().getCanonicalType();
   // The variable is a value type and we know it is only used as const. Safe
   // to reference it and avoid the copy.
   if (!isa(T))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] efa4dbc - [clang-tidy] performance-unnecessary-copy-initialization: Look at the canonical type when checking for aliases.

2021-06-09 Thread Felix Berger via cfe-commits

Author: Felix Berger
Date: 2021-06-09T16:36:53-04:00
New Revision: efa4dbc32ca9b7f3319edbcc6ac502ea962c8f0a

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

LOG: [clang-tidy] performance-unnecessary-copy-initialization: Look at the 
canonical type when checking for aliases.

This fixes a false positive case where for instance a pointer is obtained and 
declared using `auto`.

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

Reviewed-by: ymandel

Added: 


Modified: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp

clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp 
b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index 57a2310e779fb..27ce36e49a073 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -100,7 +100,7 @@ static bool isInitializingVariableImmutable(const VarDecl 
,
   if (!isOnlyUsedAsConst(InitializingVar, BlockStmt, Context))
 return false;
 
-  QualType T = InitializingVar.getType();
+  QualType T = InitializingVar.getType().getCanonicalType();
   // The variable is a value type and we know it is only used as const. Safe
   // to reference it and avoid the copy.
   if (!isa(T))

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
index 3a4c3ec869e1c..e894f84f11d8c 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -4,6 +4,7 @@ struct ExpensiveToCopyType {
   ExpensiveToCopyType();
   virtual ~ExpensiveToCopyType();
   const ExpensiveToCopyType () const;
+  const ExpensiveToCopyType *pointer() const;
   void nonConstMethod();
   bool constMethod() const;
 };
@@ -548,6 +549,25 @@ void negativeCopiedFromGetterOfReferenceToModifiedVar() {
   Orig.nonConstMethod();
 }
 
+void negativeAliasNonCanonicalPointerType() {
+  ExpensiveToCopyType Orig;
+  // The use of auto here hides that the type is a pointer type. The check 
needs
+  // to look at the canonical type to detect the aliasing through this pointer.
+  const auto Pointer = Orig.pointer();
+  const auto NecessaryCopy = Pointer->reference();
+  Orig.nonConstMethod();
+}
+
+void negativeAliasTypedefedType() {
+  typedef const ExpensiveToCopyType 
+  ExpensiveToCopyType Orig;
+  // The typedef hides the fact that this is a reference type. The check needs
+  // to look at the canonical type to detect the aliasing.
+  ReferenceType Ref = Orig.reference();
+  const auto NecessaryCopy = Ref.reference();
+  Orig.nonConstMethod();
+}
+
 void positiveCopiedFromGetterOfReferenceToConstVar() {
   ExpensiveToCopyType Orig;
   const auto  = Orig.reference();



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


[PATCH] D102901: [HWASan] Add basic stack tagging support for LAM.

2021-06-09 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse updated this revision to Diff 350989.
morehouse marked 2 inline comments as done.
morehouse added a comment.

- - Address nits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102901

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/test/hwasan/TestCases/deep-recursion.c
  compiler-rt/test/hwasan/TestCases/longjmp.c
  compiler-rt/test/hwasan/TestCases/mem-intrinsics.c
  compiler-rt/test/hwasan/TestCases/rich-stack.c
  compiler-rt/test/hwasan/TestCases/stack-history-length.c
  compiler-rt/test/hwasan/TestCases/stack-oob.c
  compiler-rt/test/hwasan/TestCases/stack-uar-dynamic.c
  compiler-rt/test/hwasan/TestCases/stack-uar-realign.c
  compiler-rt/test/hwasan/TestCases/stack-uar.c
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-array.ll
  llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-with-calls.ll
  llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca.ll
@@ -0,0 +1,45 @@
+; Test alloca instrumentation.
+;
+; RUN: opt < %s -hwasan -S | FileCheck %s --check-prefixes=CHECK,NO-UAR-TAGS
+; RUN: opt < %s -hwasan -hwasan-uar-retag-to-zero=0 -S | FileCheck %s --check-prefixes=CHECK,UAR-TAGS
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @use32(i32*)
+
+define void @test_alloca() sanitize_hwaddress {
+; CHECK-LABEL: @test_alloca(
+; CHECK: %[[FP:[^ ]*]] = call i8* @llvm.frameaddress.p0i8(i32 0)
+; CHECK: %[[A:[^ ]*]] = ptrtoint i8* %[[FP]] to i64
+; CHECK: %[[B:[^ ]*]] = lshr i64 %[[A]], 20
+; CHECK: %[[A_XOR_B:[^ ]*]] = xor i64 %[[A]], %[[B]]
+; CHECK: %[[BASE_TAG:[^ ]*]] = and i64 %[[A_XOR_B]], 63
+
+; CHECK: %[[X:[^ ]*]] = alloca { i32, [12 x i8] }, align 16
+; CHECK: %[[X_BC:[^ ]*]] = bitcast { i32, [12 x i8] }* %[[X]] to i32*
+; CHECK: %[[X_TAG:[^ ]*]] = xor i64 %[[BASE_TAG]], 0
+; CHECK: %[[X1:[^ ]*]] = ptrtoint i32* %[[X_BC]] to i64
+; CHECK: %[[C:[^ ]*]] = shl i64 %[[X_TAG]], 57
+; CHECK: %[[D:[^ ]*]] = or i64 %[[X1]], %[[C]]
+; CHECK: %[[X_HWASAN:[^ ]*]] = inttoptr i64 %[[D]] to i32*
+
+; CHECK: %[[X_TAG2:[^ ]*]] = trunc i64 %[[X_TAG]] to i8
+; CHECK: %[[X_I8:[^ ]*]] = bitcast i32* %[[X_BC]] to i8*
+; CHECK: call void @__hwasan_tag_memory(i8* %[[X_I8]], i8 %[[X_TAG2]], i64 16)
+
+; CHECK: call void @use32(i32* nonnull %[[X_HWASAN]])
+
+; UAR-TAGS: %[[BASE_TAG_COMPL:[^ ]*]] = xor i64 %[[BASE_TAG]], 63
+; UAR-TAGS: %[[X_TAG_UAR:[^ ]*]] = trunc i64 %[[BASE_TAG_COMPL]] to i8
+; CHECK: %[[X_I8_2:[^ ]*]] = bitcast i32* %[[X_BC]] to i8*
+; NO-UAR-TAGS: call void @__hwasan_tag_memory(i8* %[[X_I8_2]], i8 0, i64 16)
+; UAR-TAGS: call void @__hwasan_tag_memory(i8* %[[X_I8_2]], i8 %[[X_TAG_UAR]], i64 16)
+; CHECK: ret void
+
+
+entry:
+  %x = alloca i32, align 4
+  call void @use32(i32* nonnull %x)
+  ret void
+}
Index: llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-with-calls.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-with-calls.ll
@@ -0,0 +1,23 @@
+; Test alloca instrumentation when tags are generated by HWASan function.
+;
+; RUN: opt < %s -hwasan -hwasan-generate-tags-with-calls -S | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @use32(i32*)
+
+define void @test_alloca() sanitize_hwaddress {
+; CHECK-LABEL: @test_alloca(
+; CHECK: %[[BC:[^ ]*]] = bitcast { i32, [12 x i8] }* %x to i32*
+; CHECK: %[[T1:[^ ]*]] = call i8 @__hwasan_generate_tag()
+; CHECK: %[[A:[^ ]*]] = zext i8 %[[T1]] to i64
+; CHECK: %[[B:[^ ]*]] = ptrtoint i32* %[[BC]] to i64
+; CHECK: %[[C:[^ ]*]] = shl i64 %[[A]], 57
+; CHECK: or i64 %[[B]], %[[C]]
+
+entry:
+  %x = alloca i32, align 4
+  call void @use32(i32* nonnull %x)
+  ret void
+}
Index: llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-array.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-array.ll
@@ -0,0 +1,15 @@
+; RUN: opt < %s -hwasan -S | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @use(i8*, i8*)
+
+define void @test_alloca() sanitize_hwaddress {
+  ; CHECK: alloca { [4 x i8], [12 x i8] }, align 16
+  %x = alloca i8, i64 4
+  ; CHECK: alloca i8, i64 16, align 16
+  %y = alloca i8, i64 16
+  call void @use(i8* %x, i8* %y)
+  ret void
+}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

[PATCH] D103982: [clang] Make CXXDefaultArgExpr inherit dependence from the inner Expr

2021-06-09 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103982

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


[PATCH] D102901: [HWASan] Add basic stack tagging support for LAM.

2021-06-09 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:503
+  // planned for globals as well.
+  bool IsX86 = TargetTriple.getArch() == Triple::x86_64;
+  UsePageAliases = ClUsePageAliases && IsX86;

As is it's not technically correct



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:519
 
+  if (IsX86) {
+PointerTagShift = 57;

maybe move up and combine with other IsX86 checks about



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:964
   // 
https://github.com/google/sanitizers/blob/master/hwaddress-sanitizer/sort_masks.py
   static unsigned FastMasks[] = {0,  128, 64,  192, 32,  96,  224, 112, 240,
  48, 16,  120, 248, 56,  24,  8,   124, 252,

const


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102901

___
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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 marked 3 inline comments as done.
feg208 added a comment.

All the review comments are addressed


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] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 350984.
feg208 marked 11 inline comments as done.
feg208 added a comment.

Rolls up the remaining review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"}, // first line\n"
+   "{-1, 93463, \"world\"}, // second line\n"
+   "{ 7, 5,\"!!\"}  // third line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+   "{ 56,23, 21,   \"oh\"}, // first line\n"
+   "{ -1, 93463, 22,   \"my\"}, // second line\n"
+   "{  7, 5,  1, \"goodness\"}  // third line\n"
+   "{234, 5,  1, \"gracious\"}  // fourth line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{int{56},23, \"hello\"},\n"
+   "{int{-1}, 93463, \"world\"},\n"
+   "{ int{7}, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("demo = std::array{\n"
+   "test{56,23, \"hello\"},\n"
+   "test{-1, 93463, \"world\"},\n"
+   "test{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "#if X\n"
+   "{-1, 93463, \"world\"},\n"
+   "#endif\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat(
+  "test demo[] = {\n"
+  "{ 7,23,\n"
+  " \"hello world i am a very long line that really, in any\"\n"
+  " \"just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  \"world\"},\n"
+  "{56, 5, \"!!\"}\n"
+  "};\n",
+  Style);
+
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+   "{{\"sign\"}, \"Sign\",  "
+   "  {\"x\", \"dy\"}},\n"
+   "{  {\"dx\"},  \"Mul\", {\"dy\""
+   ", \"sign\"}},\n"
+   "});\n",
+   Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+  "test demo[] = {\n"
+  "{56,23, \"hello world i am a very long line that really, "
+  "in any just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  "
+  " \"world\"},\n"
+  "{ 7, 5,  "
+  "\"!!\"},\n"
+  "};",
+  format("test demo[] = {{56, 23, \"hello 

[PATCH] D103987: Start tracking Clang's C implementation status

2021-06-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, erichkeane, jyknight.
aaron.ballman requested review of this revision.
Herald added a project: clang.

We currently have a very useful page tracking our implementation status of C++ 
features (https://clang.llvm.org/cxx_status.html) but we have nothing similar 
for C. This patch adds the initial support for tracking this information. It's 
a work-in-progress.

Currently, the document is generated by hand from the information listed in the 
latest C2x draft (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2596.pdf for 
the moment) on the papers that directly impact the language (as opposed to 
library features). This means information is missing about all the C versions 
before C2x and it is missing the papers that have been accepted but not yet 
rolled into the working draft.

I verified the proposals that I believe Clang definitively does/does not 
implement, but I marked a number of papers as "unknown" because I've not had 
the chance to see what support we have for them yet. I expect the "Unknown" 
columns to dwindle as I get the chance to check their status more thoroughly. I 
also expect we'll add papers from C17 and potentially C11, but I am not certain 
I have the bandwidth to go as far back in time as C99 let alone C89. Once the 
status page for features is done, a future endeavor can add a DR status page.

Assuming that this status page is desirable to others, I figure we can commit 
the incomplete page and I can add information to it post-commit, as I have the 
time. One question this work did raise for me is whether we want to update the 
bug tracker to have specific C versions like we do for C++ (currently, we 
categorize by standard version for C++ and just use "C" as a catch-all for all 
the versions of C)?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103987

Files:
  clang/www/c_status.html
  clang/www/menu.html.incl

Index: clang/www/menu.html.incl
===
--- clang/www/menu.html.incl
+++ clang/www/menu.html.incl
@@ -12,6 +12,7 @@
 User's Manual
 LanguageCompatibility
 LanguageExtensions
+C Status
 C++ Status
   
 
Index: clang/www/c_status.html
===
--- /dev/null
+++ clang/www/c_status.html
@@ -0,0 +1,296 @@
+
+
+
+  
+  Clang - C Programming Language Status
+  
+  
+  
+.none { background-color: #FF }
+.partial { background-color: #FFE0B0 }
+.unreleased { background-color: #99 }
+.unknown { background-color: #FF55FF }
+.full { background-color: #CCFF99 }
+.na { background-color: #DD }
+:target { background-color: #BB; outline: #55 solid thin; }
+th { background-color: #FFDDAA }
+td { vertical-align: middle }
+tt { white-space: nowrap }
+  
+
+
+
+
+
+
+
+
+C Support in Clang
+
+
+Clang implements the following published and upcoming ISO C standards:
+
+
+
+ Language Standard
+ Flag
+ Available in Clang?
+
+
+ C89
+ -std=c89
+ Yes
+
+
+ C99
+ -std=c99
+ Almost certainly
+
+
+ C11
+ -std=c11
+ Probably
+
+
+ C17
+ -std=c17
+ Maybe?
+
+
+ C2x
+ -std=c2x
+ Partial
+
+
+
+The Clang community is continually striving to improve C standards
+compliance between releases. We implement the resolution for defect
+reports, but we do not currently track our DR status (help with
+tracking DR status is appreciated).
+
+The https://bugs.llvm.org/;>LLVM bug tracker contains a
+Clang C component that tracks known bugs with Clang's language
+conformance.
+
+C89 implementation status
+
+
+
+C99 implementation status
+
+
+
+C11 implementation status
+
+
+
+C17 implementation status
+
+
+
+C2x implementation status
+
+
+
+You can use Clang in C2x mode with the -std=c2x option.
+
+
+List of features and minimum Clang version with support
+
+
+ 
+Language Feature
+C2x Proposal
+Available in Clang?
+ 
+
+
+  Evaluation formats
+  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2186.pdf;>N2186
+  Unknown
+
+
+  Clarifying the restrict Keyword v2
+  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2260.pdf;>N2660
+  Unknown
+
+
+  Harmonizing static_assert with C++
+  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2265.pdf;>N2665
+  Clang 9
+
+
+  nodiscard attribute
+  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2667.pdf;>N2667
+  Clang 9
+
+
+  maybe_unused attribute
+  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2670.pdf;>N2670
+  Clang 9
+
+
+  TS 18661 Integration
+
+   
+http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2314.pdf;>N2314
+Unknown
+  
+   
+http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2341.pdf;>N2341
+Unknown
+  
+   
+http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2401.pdf;>N2401
+Unknown

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

2021-06-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

In D103958#2808861 , @efriedma wrote:

>> without resorting to inline assembly (which often results in poor codegen).
>
> Could you give an example of the resulting assembly with mustcontrol vs. this 
> patch?

Err, I mean, the resulting assembly using the inline asm version, vs. an 
equivalent construct using mustcontrol.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103958

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


[PATCH] D103958: [WIP] Support MustControl conditional control attribute

2021-06-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

I don't like using metadata like this.  Dropping metadata should generally 
preserve the semantics of the code.

> without resorting to inline assembly (which often results in poor codegen).

Could you give an example of the resulting assembly with mustcontrol vs. this 
patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103958

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


[PATCH] D102175: [clang-tidy] performance-unnecessary-copy-initialization: Remove the complete statement when the copied variable is unused.

2021-06-09 Thread Felix Berger 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 rG5dbe3bf4b8db: [clang-tidy] 
performance-unnecessary-copy-initialization: Remove the complete… (authored by 
flx).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102175

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization-allowed-types.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -38,60 +38,88 @@
   const auto AutoAssigned = ExpensiveTypeReference();
   // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
   // CHECK-FIXES: const auto& AutoAssigned = ExpensiveTypeReference();
+  AutoAssigned.constMethod();
+
   const auto AutoCopyConstructed(ExpensiveTypeReference());
   // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
   // CHECK-FIXES: const auto& AutoCopyConstructed(ExpensiveTypeReference());
+  AutoCopyConstructed.constMethod();
+
   const ExpensiveToCopyType VarAssigned = ExpensiveTypeReference();
   // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
   // CHECK-FIXES:   const ExpensiveToCopyType& VarAssigned = ExpensiveTypeReference();
+  VarAssigned.constMethod();
+
   const ExpensiveToCopyType VarCopyConstructed(ExpensiveTypeReference());
   // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed'
   // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(ExpensiveTypeReference());
+  VarCopyConstructed.constMethod();
 }
 
 void PositiveMethodCallConstReferenceParam(const ExpensiveToCopyType ) {
   const auto AutoAssigned = Obj.reference();
   // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
   // CHECK-FIXES: const auto& AutoAssigned = Obj.reference();
+  AutoAssigned.constMethod();
+
   const auto AutoCopyConstructed(Obj.reference());
   // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
   // CHECK-FIXES: const auto& AutoCopyConstructed(Obj.reference());
+  AutoCopyConstructed.constMethod();
+
   const ExpensiveToCopyType VarAssigned = Obj.reference();
   // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
   // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = Obj.reference();
+  VarAssigned.constMethod();
+
   const ExpensiveToCopyType VarCopyConstructed(Obj.reference());
   // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed'
   // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(Obj.reference());
+  VarCopyConstructed.constMethod();
 }
 
 void PositiveMethodCallConstParam(const ExpensiveToCopyType Obj) {
   const auto AutoAssigned = Obj.reference();
   // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
   // CHECK-FIXES: const auto& AutoAssigned = Obj.reference();
+  AutoAssigned.constMethod();
+
   const auto AutoCopyConstructed(Obj.reference());
   // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
   // CHECK-FIXES: const auto& AutoCopyConstructed(Obj.reference());
+  AutoCopyConstructed.constMethod();
+
   const ExpensiveToCopyType VarAssigned = Obj.reference();
   // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
   // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = Obj.reference();
+  VarAssigned.constMethod();
+
   const ExpensiveToCopyType VarCopyConstructed(Obj.reference());
   // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed'
   // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(Obj.reference());
+  VarCopyConstructed.constMethod();
 }
 
 void PositiveMethodCallConstPointerParam(const ExpensiveToCopyType *const Obj) {
   const auto AutoAssigned = Obj->reference();
   // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
   // CHECK-FIXES: const auto& AutoAssigned = Obj->reference();
+  AutoAssigned.constMethod();
+
   const auto AutoCopyConstructed(Obj->reference());
   // CHECK-MESSAGES: 

[clang-tools-extra] 5dbe3bf - [clang-tidy] performance-unnecessary-copy-initialization: Remove the complete statement when the copied variable is unused.

2021-06-09 Thread Felix Berger via cfe-commits

Author: Felix Berger
Date: 2021-06-09T15:52:48-04:00
New Revision: 5dbe3bf4b8dbb7e67d41c7c1360f15d512dd72a0

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

LOG: [clang-tidy] performance-unnecessary-copy-initialization: Remove the 
complete statement when the copied variable is unused.

It is not useful to keep the statement around and can lead to compiler
warnings when -Wall (-Wunused-variable specifically) turned on.

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

Reviewed-by: ymandel

Added: 


Modified: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h

clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization-allowed-types.cpp

clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp 
b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index 0cc9a44263021..57a2310e779fb 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -7,9 +7,9 @@
 
//===--===//
 
 #include "UnnecessaryCopyInitialization.h"
-
 #include "../utils/DeclRefExprUtils.h"
 #include "../utils/FixItHintUtils.h"
+#include "../utils/LexerUtils.h"
 #include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/Basic/Diagnostic.h"
@@ -21,6 +21,7 @@ namespace {
 
 using namespace ::clang::ast_matchers;
 using llvm::StringRef;
+using utils::decl_ref_expr::allDeclRefExprs;
 using utils::decl_ref_expr::isOnlyUsedAsConst;
 
 static constexpr StringRef ObjectArgId = "objectArg";
@@ -37,6 +38,19 @@ void recordFixes(const VarDecl , ASTContext ,
   }
 }
 
+void recordRemoval(const DeclStmt , ASTContext ,
+   DiagnosticBuilder ) {
+  // Attempt to remove the whole line until the next non-comment token.
+  auto Tok = utils::lexer::findNextTokenSkippingComments(
+  Stmt.getEndLoc(), Context.getSourceManager(), Context.getLangOpts());
+  if (Tok) {
+Diagnostic << FixItHint::CreateRemoval(SourceRange(
+Stmt.getBeginLoc(), Tok->getLocation().getLocWithOffset(-1)));
+  } else {
+Diagnostic << FixItHint::CreateRemoval(Stmt.getSourceRange());
+  }
+}
+
 AST_MATCHER_FUNCTION(StatementMatcher, isConstRefReturningMethodCall) {
   // Match method call expressions where the `this` argument is only used as
   // const, this will be checked in `check()` part. This returned const
@@ -118,6 +132,11 @@ static bool isInitializingVariableImmutable(const VarDecl 
,
   return false;
 }
 
+bool isVariableUnused(const VarDecl , const Stmt ,
+  ASTContext ) {
+  return allDeclRefExprs(Var, BlockStmt, Context).empty();
+}
+
 } // namespace
 
 UnnecessaryCopyInitialization::UnnecessaryCopyInitialization(
@@ -169,14 +188,13 @@ void UnnecessaryCopyInitialization::check(
   const auto *ObjectArg = Result.Nodes.getNodeAs(ObjectArgId);
   const auto *BlockStmt = Result.Nodes.getNodeAs("blockStmt");
   const auto *CtorCall = Result.Nodes.getNodeAs("ctorCall");
+  const auto *Stmt = Result.Nodes.getNodeAs("declStmt");
 
   TraversalKindScope RAII(*Result.Context, TK_AsIs);
 
   // Do not propose fixes if the DeclStmt has multiple VarDecls or in macros
   // since we cannot place them correctly.
-  bool IssueFix =
-  Result.Nodes.getNodeAs("declStmt")->isSingleDecl() &&
-  !NewVar->getLocation().isMacroID();
+  bool IssueFix = Stmt->isSingleDecl() && !NewVar->getLocation().isMacroID();
 
   // A constructor that looks like T(const T& t, bool arg = false) counts as a
   // copy only when it is called with default arguments for the arguments after
@@ -186,47 +204,68 @@ void UnnecessaryCopyInitialization::check(
   return;
 
   if (OldVar == nullptr) {
-handleCopyFromMethodReturn(*NewVar, *BlockStmt, IssueFix, ObjectArg,
+handleCopyFromMethodReturn(*NewVar, *BlockStmt, *Stmt, IssueFix, ObjectArg,
*Result.Context);
   } else {
-handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, IssueFix,
+handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, *Stmt, IssueFix,
*Result.Context);
   }
 }
 
 void UnnecessaryCopyInitialization::handleCopyFromMethodReturn(
-const VarDecl , const Stmt , bool IssueFix,
-const VarDecl *ObjectArg, ASTContext ) {
+const VarDecl , const Stmt , const DeclStmt ,
+bool IssueFix, const VarDecl *ObjectArg, ASTContext ) {
   bool IsConstQualified = 

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 marked an inline comment as done.
feg208 added a comment.

In D101868#2808826 , @curdeius wrote:

> LGTM. That's a great piece work @feg208. Thank you!

Awww thanks. I learned a lot from all the comments honestly. I appreciate the 
patience.

> I've added many nit comments, but I didn't do it for all code comments.
> Please check that all comments are full phrases (with full stops :) ) before 
> landing.
> Some comments are in .rst but you know that you need to update Format.h and 
> then regenerate .rst :).
> Also, don't hesitate to mark comments as done.

I'll roll these up.

> Do you need somebody to land it on your behalf?

I do. I don't have commit rights


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] D102175: [clang-tidy] performance-unnecessary-copy-initialization: Remove the complete statement when the copied variable is unused.

2021-06-09 Thread Felix Berger via Phabricator via cfe-commits
flx added a comment.

Thank you for the review, Yitzhak!




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:541
   Orig.constMethod();
+  UnnecessaryCopy.constMethod();
 }

ymandel wrote:
> If this line weren't here, then we'd delete line 537, in which case `Ref` 
> itself becomes unused and so line 536 should be deleted as well. Do you 
> handle this case?
Good catch. This case is not handled. It would require recursively checking 
whether the deleted statement was the only reference to another reference which 
is hard and requires subtracting that decl ref from all these calculations.  

 I would like to to see this being a common enough issue before addressing it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102175

___
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

2021-06-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. That's a great piece work @feg208. Thank you!

I've added many nit comments, but I didn't do it for all code comments.
Please check that all comments are full phrases (with full stops :) ) before 
landing.
Some comments are in .rst but you know that you need to update Format.h and 
then regenerate .rst :).
Also, don't hesitate to mark comments as done.

Do you need somebody to land it on your behalf?




Comment at: clang/docs/ClangFormatStyleOptions.rst:213-218
+struct test demo[] =
+{
+{56,23, "hello"},
+{-1, 93463, "world"},
+{ 7, 5,"!!"}
+}

This repetition of `demo` struct seems not very useful.



Comment at: clang/docs/ClangFormatStyleOptions.rst:247
+  * ``AIAS_None`` (in configuration: ``None``)
+Don't align array initializer columns
+

Nit. This can be done when landing. No need to update the patch just for this.



Comment at: clang/include/clang/Format/Format.h:93
 
+  /// Different style for aligning array initializers
+  enum ArrayInitializerAlignmentStyle {





Comment at: clang/lib/Format/FormatToken.h:434-442
+  /// The first token in set of column elements
+  bool StartsColumn = false;
+
+  /// This notes the start of the line of an array initializer
+  bool ArrayInitializerLineStart = false;
+
+  /// This starts an array initializer

Nits.



Comment at: clang/lib/Format/WhitespaceManager.cpp:994
+
+  // Now go through and fixup the spaces
+  auto *CellIter = Cells.begin();





Comment at: clang/lib/Format/WhitespaceManager.cpp:1005
+  // on a line that was split. If so on that line we make sure that
+  // the spaces in front of the brace are enough
+  Changes[CellIter->Index].NewlinesBefore = 0;





Comment at: clang/lib/Format/WhitespaceManager.cpp:1013-1014
+  }
+  // Except if the array is empty we need the position of all the
+  // cells immediantly adjacent to this
+  if (CellIter != Cells.begin()) {





Comment at: clang/lib/Format/WhitespaceManager.cpp:1062
+
+  // Now go through and fixup the spaces
+  auto *CellIter = Cells.begin();





Comment at: clang/lib/Format/WhitespaceManager.cpp:1064
+  auto *CellIter = Cells.begin();
+  // The first cell needs to be against the left brace
+  if (Changes[CellIter->Index].NewlinesBefore == 0)





Comment at: clang/lib/Format/WhitespaceManager.h:185-190
+constexpr bool operator==(const CellDescription ) const {
+  return Index == oth.Index && Cell == oth.Cell && EndIndex == 
oth.EndIndex;
+}
+constexpr bool operator!=(const CellDescription ) const {
+  return !(*this == oth);
+}

Nit.



Comment at: clang/lib/Format/WhitespaceManager.h:265
+  // If we broke the line the initial spaces are already
+  // accounted for
+  if (Changes[PrevIter->Index].NewlinesBefore > 0)

Nit.


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] D103986: [PowerPC] Floating Point Builtins for XL Compat.

2021-06-09 Thread Quinn Pham via Phabricator via cfe-commits
quinnp created this revision.
Herald added subscribers: shchenz, kbarton, hiraditya, nemanjai.
quinnp requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103986

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-ppc-xlcompat-fp.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  llvm/lib/Target/PowerPC/PPCInstrVSX.td
  llvm/test/CodeGen/builtins-ppc-xlcompat-fp.ll

Index: llvm/test/CodeGen/builtins-ppc-xlcompat-fp.ll
===
--- /dev/null
+++ llvm/test/CodeGen/builtins-ppc-xlcompat-fp.ll
@@ -0,0 +1,74 @@
+; RUN: llc -verify-machineinstrs -mtriple=powerpcle-unknown-linux-gnu \
+; RUN: -ppc-asm-full-reg-names -mcpu=pwr7 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-linux-gnu \
+; RUN: -ppc-asm-full-reg-names -mcpu=pwr7 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN: -ppc-asm-full-reg-names -mcpu=pwr7 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN: -ppc-asm-full-reg-names -mcpu=pwr7 < %s | FileCheck %s
+
+define dso_local double @test_fsel(double %a, double %b, double %c) local_unnamed_addr #0 {
+; CHECK-LABEL: test_fsel 
+
+entry:
+  %0 = tail call double @llvm.ppc.fsel(double %a, double %b, double %c)
+; CHECK: fsel f1, f1, f2, f3
+
+  ret double %0
+; CHECK: blr
+}
+
+declare double @llvm.ppc.fsel(double, double, double) #2
+
+define dso_local float @test_fsels(float %a, float %b, float %c) local_unnamed_addr #0 {
+; CHECK-LABEL: test_fsels
+
+entry:
+  %0 = tail call float @llvm.ppc.fsels(float %a, float %b, float %c)
+; CHECK: fsel f1, f1, f2, f3
+
+  ret float %0
+; CHECK: blr
+}
+
+declare float @llvm.ppc.fsels(float, float, float) #2
+
+define dso_local double @test_frsqrte(double %a) local_unnamed_addr #0 {
+; CHECK-LABEL: test_frsqrte
+
+entry:
+  %0 = tail call double @llvm.ppc.frsqrte(double %a)
+; CHECK: xsrsqrtedp f1, f1
+
+  ret double %0
+; CHECK: blr
+}
+
+declare double @llvm.ppc.frsqrte(double) #2
+
+define dso_local float @test_frsqrtes(float %a) local_unnamed_addr #0 {
+; CHECK-LABEL: test_frsqrtes
+
+entry:
+  %0 = tail call float @llvm.ppc.frsqrtes(float %a)
+; CHECK: frsqrtes f1, f1
+ 
+  ret float %0
+; CHECK: blr
+}
+
+declare float @llvm.ppc.frsqrtes(float) #2
+
+define dso_local float @test_fsqrts(float %a) local_unnamed_addr #0 {
+; CHECK-LABEL: test_fsqrts
+
+entry:
+  %0 = tail call float @llvm.ppc.fsqrts(float %a)
+; CHECK fsqrts f1, f1
+
+  ret float %0
+; CHECK: blr
+}
+
+declare float @llvm.ppc.fsqrts(float) #2
+
Index: llvm/lib/Target/PowerPC/PPCInstrVSX.td
===
--- llvm/lib/Target/PowerPC/PPCInstrVSX.td
+++ llvm/lib/Target/PowerPC/PPCInstrVSX.td
@@ -4954,6 +4954,8 @@
 } // HasVSX, IsISA3_0, HasDirectMove, IsLittleEndian
 } // AddedComplexity = 400
 
+def : Pat<(int_ppc_frsqrte vsfrc:$XB), (XSRSQRTEDP $XB)>;
+
 // Instruction aliases ---//
 def : InstAlias<"xvmovdp $XT, $XB",
 (XVCPSGNDP vsrc:$XT, vsrc:$XB, vsrc:$XB)>;
Index: llvm/lib/Target/PowerPC/PPCInstrInfo.td
===
--- llvm/lib/Target/PowerPC/PPCInstrInfo.td
+++ llvm/lib/Target/PowerPC/PPCInstrInfo.td
@@ -3318,6 +3318,10 @@
   }
 }
 
+def : Pat<(int_ppc_fsel f8rc:$FRA, f8rc:$FRC, f8rc:$FRB), (FSELD $FRA, $FRC, $FRB)>;
+def : Pat<(int_ppc_frsqrtes f4rc:$frB), (FRSQRTES $frB)>;  
+def : Pat<(int_ppc_fsqrts f4rc:$frB), (FSQRTS $frB)>;
+
 let hasSideEffects = 0 in {
 let PPC970_Unit = 1 in {  // FXU Operations.
   let isSelect = 1 in
Index: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
+++ llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
@@ -4987,6 +4987,13 @@
 break;
 
   case ISD::INTRINSIC_WO_CHAIN: {
+
+if (N->getConstantOperandVal(0) == Intrinsic::ppc_fsels) {
+  SDValue ops[] = {N->getOperand(1), N->getOperand(2), N->getOperand(3)};
+  CurDAG->SelectNodeTo(N, PPC::FSELS, MVT::f32, ops);
+  return;
+}
+
 if (!Subtarget->isISA3_1())
   break;
 unsigned Opcode = 0;
Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -1523,5 +1523,18 @@
   Intrinsic<[],[],[]>;
   def int_ppc_iospace_eieio : GCCBuiltin<"__builtin_ppc_iospace_eieio">,
   Intrinsic<[],[],[]>;
+
+  def 

[PATCH] D102901: [HWASan] Add basic stack tagging support for LAM.

2021-06-09 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse updated this revision to Diff 350976.
morehouse added a comment.

- Privatize new member variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102901

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/test/hwasan/TestCases/deep-recursion.c
  compiler-rt/test/hwasan/TestCases/longjmp.c
  compiler-rt/test/hwasan/TestCases/mem-intrinsics.c
  compiler-rt/test/hwasan/TestCases/rich-stack.c
  compiler-rt/test/hwasan/TestCases/stack-history-length.c
  compiler-rt/test/hwasan/TestCases/stack-oob.c
  compiler-rt/test/hwasan/TestCases/stack-uar-dynamic.c
  compiler-rt/test/hwasan/TestCases/stack-uar-realign.c
  compiler-rt/test/hwasan/TestCases/stack-uar.c
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-array.ll
  llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-with-calls.ll
  llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca.ll
@@ -0,0 +1,45 @@
+; Test alloca instrumentation.
+;
+; RUN: opt < %s -hwasan -S | FileCheck %s --check-prefixes=CHECK,NO-UAR-TAGS
+; RUN: opt < %s -hwasan -hwasan-uar-retag-to-zero=0 -S | FileCheck %s --check-prefixes=CHECK,UAR-TAGS
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @use32(i32*)
+
+define void @test_alloca() sanitize_hwaddress {
+; CHECK-LABEL: @test_alloca(
+; CHECK: %[[FP:[^ ]*]] = call i8* @llvm.frameaddress.p0i8(i32 0)
+; CHECK: %[[A:[^ ]*]] = ptrtoint i8* %[[FP]] to i64
+; CHECK: %[[B:[^ ]*]] = lshr i64 %[[A]], 20
+; CHECK: %[[A_XOR_B:[^ ]*]] = xor i64 %[[A]], %[[B]]
+; CHECK: %[[BASE_TAG:[^ ]*]] = and i64 %[[A_XOR_B]], 63
+
+; CHECK: %[[X:[^ ]*]] = alloca { i32, [12 x i8] }, align 16
+; CHECK: %[[X_BC:[^ ]*]] = bitcast { i32, [12 x i8] }* %[[X]] to i32*
+; CHECK: %[[X_TAG:[^ ]*]] = xor i64 %[[BASE_TAG]], 0
+; CHECK: %[[X1:[^ ]*]] = ptrtoint i32* %[[X_BC]] to i64
+; CHECK: %[[C:[^ ]*]] = shl i64 %[[X_TAG]], 57
+; CHECK: %[[D:[^ ]*]] = or i64 %[[X1]], %[[C]]
+; CHECK: %[[X_HWASAN:[^ ]*]] = inttoptr i64 %[[D]] to i32*
+
+; CHECK: %[[X_TAG2:[^ ]*]] = trunc i64 %[[X_TAG]] to i8
+; CHECK: %[[X_I8:[^ ]*]] = bitcast i32* %[[X_BC]] to i8*
+; CHECK: call void @__hwasan_tag_memory(i8* %[[X_I8]], i8 %[[X_TAG2]], i64 16)
+
+; CHECK: call void @use32(i32* nonnull %[[X_HWASAN]])
+
+; UAR-TAGS: %[[BASE_TAG_COMPL:[^ ]*]] = xor i64 %[[BASE_TAG]], 63
+; UAR-TAGS: %[[X_TAG_UAR:[^ ]*]] = trunc i64 %[[BASE_TAG_COMPL]] to i8
+; CHECK: %[[X_I8_2:[^ ]*]] = bitcast i32* %[[X_BC]] to i8*
+; NO-UAR-TAGS: call void @__hwasan_tag_memory(i8* %[[X_I8_2]], i8 0, i64 16)
+; UAR-TAGS: call void @__hwasan_tag_memory(i8* %[[X_I8_2]], i8 %[[X_TAG_UAR]], i64 16)
+; CHECK: ret void
+
+
+entry:
+  %x = alloca i32, align 4
+  call void @use32(i32* nonnull %x)
+  ret void
+}
Index: llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-with-calls.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-with-calls.ll
@@ -0,0 +1,23 @@
+; Test alloca instrumentation when tags are generated by HWASan function.
+;
+; RUN: opt < %s -hwasan -hwasan-generate-tags-with-calls -S | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @use32(i32*)
+
+define void @test_alloca() sanitize_hwaddress {
+; CHECK-LABEL: @test_alloca(
+; CHECK: %[[BC:[^ ]*]] = bitcast { i32, [12 x i8] }* %x to i32*
+; CHECK: %[[T1:[^ ]*]] = call i8 @__hwasan_generate_tag()
+; CHECK: %[[A:[^ ]*]] = zext i8 %[[T1]] to i64
+; CHECK: %[[B:[^ ]*]] = ptrtoint i32* %[[BC]] to i64
+; CHECK: %[[C:[^ ]*]] = shl i64 %[[A]], 57
+; CHECK: or i64 %[[B]], %[[C]]
+
+entry:
+  %x = alloca i32, align 4
+  call void @use32(i32* nonnull %x)
+  ret void
+}
Index: llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-array.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-array.ll
@@ -0,0 +1,15 @@
+; RUN: opt < %s -hwasan -S | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @use(i8*, i8*)
+
+define void @test_alloca() sanitize_hwaddress {
+  ; CHECK: alloca { [4 x i8], [12 x i8] }, align 16
+  %x = alloca i8, i64 4
+  ; CHECK: alloca i8, i64 16, align 16
+  %y = alloca i8, i64 16
+  call void @use(i8* %x, i8* %y)
+  ret void
+}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- 

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

2021-06-09 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D103958#2808767 , @nickdesaulniers 
wrote:

> The first talk from https://www.youtube.com/watch?v=FFjV9f_Ub9o 
> (https://github.com/ClangBuiltLinux/plumbers-2020-slides/blob/master/LPC_2020_--_Dependency_ordering.pdf)
>  might be helpful to link to at some point from the commit message, for a 
> little additional context.

I read the slides and I'm not sure how this connects. I'll wait for the LangRef 
and/or IR example :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103958

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


[PATCH] D102901: [HWASan] Add basic stack tagging support for LAM.

2021-06-09 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse updated this revision to Diff 350975.
morehouse marked 3 inline comments as done.
morehouse added a comment.
Herald added a project: clang.
Herald added subscribers: Sanitizers, cfe-commits.

- Rename flag as experimental.
- Refactor and simplify code.
- Apply mask to base tag only.
- Enable some compiler-rt stack tests on x86.
- Add IR tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102901

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/test/hwasan/TestCases/deep-recursion.c
  compiler-rt/test/hwasan/TestCases/longjmp.c
  compiler-rt/test/hwasan/TestCases/mem-intrinsics.c
  compiler-rt/test/hwasan/TestCases/rich-stack.c
  compiler-rt/test/hwasan/TestCases/stack-history-length.c
  compiler-rt/test/hwasan/TestCases/stack-oob.c
  compiler-rt/test/hwasan/TestCases/stack-uar-dynamic.c
  compiler-rt/test/hwasan/TestCases/stack-uar-realign.c
  compiler-rt/test/hwasan/TestCases/stack-uar.c
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-array.ll
  llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-with-calls.ll
  llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca.ll
@@ -0,0 +1,45 @@
+; Test alloca instrumentation.
+;
+; RUN: opt < %s -hwasan -S | FileCheck %s --check-prefixes=CHECK,NO-UAR-TAGS
+; RUN: opt < %s -hwasan -hwasan-uar-retag-to-zero=0 -S | FileCheck %s --check-prefixes=CHECK,UAR-TAGS
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @use32(i32*)
+
+define void @test_alloca() sanitize_hwaddress {
+; CHECK-LABEL: @test_alloca(
+; CHECK: %[[FP:[^ ]*]] = call i8* @llvm.frameaddress.p0i8(i32 0)
+; CHECK: %[[A:[^ ]*]] = ptrtoint i8* %[[FP]] to i64
+; CHECK: %[[B:[^ ]*]] = lshr i64 %[[A]], 20
+; CHECK: %[[A_XOR_B:[^ ]*]] = xor i64 %[[A]], %[[B]]
+; CHECK: %[[BASE_TAG:[^ ]*]] = and i64 %[[A_XOR_B]], 63
+
+; CHECK: %[[X:[^ ]*]] = alloca { i32, [12 x i8] }, align 16
+; CHECK: %[[X_BC:[^ ]*]] = bitcast { i32, [12 x i8] }* %[[X]] to i32*
+; CHECK: %[[X_TAG:[^ ]*]] = xor i64 %[[BASE_TAG]], 0
+; CHECK: %[[X1:[^ ]*]] = ptrtoint i32* %[[X_BC]] to i64
+; CHECK: %[[C:[^ ]*]] = shl i64 %[[X_TAG]], 57
+; CHECK: %[[D:[^ ]*]] = or i64 %[[X1]], %[[C]]
+; CHECK: %[[X_HWASAN:[^ ]*]] = inttoptr i64 %[[D]] to i32*
+
+; CHECK: %[[X_TAG2:[^ ]*]] = trunc i64 %[[X_TAG]] to i8
+; CHECK: %[[X_I8:[^ ]*]] = bitcast i32* %[[X_BC]] to i8*
+; CHECK: call void @__hwasan_tag_memory(i8* %[[X_I8]], i8 %[[X_TAG2]], i64 16)
+
+; CHECK: call void @use32(i32* nonnull %[[X_HWASAN]])
+
+; UAR-TAGS: %[[BASE_TAG_COMPL:[^ ]*]] = xor i64 %[[BASE_TAG]], 63
+; UAR-TAGS: %[[X_TAG_UAR:[^ ]*]] = trunc i64 %[[BASE_TAG_COMPL]] to i8
+; CHECK: %[[X_I8_2:[^ ]*]] = bitcast i32* %[[X_BC]] to i8*
+; NO-UAR-TAGS: call void @__hwasan_tag_memory(i8* %[[X_I8_2]], i8 0, i64 16)
+; UAR-TAGS: call void @__hwasan_tag_memory(i8* %[[X_I8_2]], i8 %[[X_TAG_UAR]], i64 16)
+; CHECK: ret void
+
+
+entry:
+  %x = alloca i32, align 4
+  call void @use32(i32* nonnull %x)
+  ret void
+}
Index: llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-with-calls.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-with-calls.ll
@@ -0,0 +1,23 @@
+; Test alloca instrumentation when tags are generated by HWASan function.
+;
+; RUN: opt < %s -hwasan -hwasan-generate-tags-with-calls -S | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @use32(i32*)
+
+define void @test_alloca() sanitize_hwaddress {
+; CHECK-LABEL: @test_alloca(
+; CHECK: %[[BC:[^ ]*]] = bitcast { i32, [12 x i8] }* %x to i32*
+; CHECK: %[[T1:[^ ]*]] = call i8 @__hwasan_generate_tag()
+; CHECK: %[[A:[^ ]*]] = zext i8 %[[T1]] to i64
+; CHECK: %[[B:[^ ]*]] = ptrtoint i32* %[[BC]] to i64
+; CHECK: %[[C:[^ ]*]] = shl i64 %[[A]], 57
+; CHECK: or i64 %[[B]], %[[C]]
+
+entry:
+  %x = alloca i32, align 4
+  call void @use32(i32* nonnull %x)
+  ret void
+}
Index: llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-array.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/X86/alloca-array.ll
@@ -0,0 +1,15 @@
+; RUN: opt < %s -hwasan -S | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @use(i8*, i8*)
+
+define void @test_alloca() sanitize_hwaddress {
+  ; CHECK: alloca { [4 x i8], [12 x i8] }, align 16
+  %x = alloca i8, i64 4
+  ; CHECK: alloca 

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 marked an inline comment as done.
feg208 added a comment.

addressed the remaining review comment




Comment at: clang/docs/ClangFormatStyleOptions.rst:216-221
+struct test demo[] =
+{
+{56,23, "hello"},
+{-1, 93463, "world"},
+{ 7, 5,"!!"}
+};

curdeius wrote:
> feg208 wrote:
> > curdeius wrote:
> > > Don't forget to re-generate .rst.
> > I am sorry. I am not entirely sure what to check here? Should I regenerate 
> > the docs for the site and just ensure that they look ok?
> It seems that you modified Format.h but didn't re-run 
> `clang/docs/tools/dump_format_style.py` (example for `Left` is actually 
> right-aligned).
Got it. Sorry for the confusion.


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] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 350974.
feg208 added a comment.

Regenerated the mangled ClangFormatStyle.rst file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"}, // first line\n"
+   "{-1, 93463, \"world\"}, // second line\n"
+   "{ 7, 5,\"!!\"}  // third line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+   "{ 56,23, 21,   \"oh\"}, // first line\n"
+   "{ -1, 93463, 22,   \"my\"}, // second line\n"
+   "{  7, 5,  1, \"goodness\"}  // third line\n"
+   "{234, 5,  1, \"gracious\"}  // fourth line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{int{56},23, \"hello\"},\n"
+   "{int{-1}, 93463, \"world\"},\n"
+   "{ int{7}, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("demo = std::array{\n"
+   "test{56,23, \"hello\"},\n"
+   "test{-1, 93463, \"world\"},\n"
+   "test{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "#if X\n"
+   "{-1, 93463, \"world\"},\n"
+   "#endif\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat(
+  "test demo[] = {\n"
+  "{ 7,23,\n"
+  " \"hello world i am a very long line that really, in any\"\n"
+  " \"just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  \"world\"},\n"
+  "{56, 5, \"!!\"}\n"
+  "};\n",
+  Style);
+
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+   "{{\"sign\"}, \"Sign\",  "
+   "  {\"x\", \"dy\"}},\n"
+   "{  {\"dx\"},  \"Mul\", {\"dy\""
+   ", \"sign\"}},\n"
+   "});\n",
+   Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+  "test demo[] = {\n"
+  "{56,23, \"hello world i am a very long line that really, "
+  "in any just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  "
+  " \"world\"},\n"
+  "{ 7, 5,  "
+  "\"!!\"},\n"
+  "};",
+  format("test demo[] = {{56, 23, \"hello world i am a very long line "
+ 

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

2021-06-09 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

The first talk from https://www.youtube.com/watch?v=FFjV9f_Ub9o 
(https://github.com/ClangBuiltLinux/plumbers-2020-slides/blob/master/LPC_2020_--_Dependency_ordering.pdf)
 might be helpful to link to at some point from the commit message, for a 
little additional context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103958

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


[PATCH] D101139: Create install targets for scan-build-py.

2021-06-09 Thread Daniel via Phabricator via cfe-commits
isthismyaccount added a comment.

Thanks for the reviews. I'll take over this and try to get the merge conflicts 
resolved.


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

https://reviews.llvm.org/D101139

___
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

2021-06-09 Thread Gabor Marton via Phabricator via cfe-commits
martong marked an inline comment as done.
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1789
+  if (DisequalToOther.contains(*this))
+return nullptr;
   if (!DisequalToOther.isEmpty()) {

vsavchenko wrote:
> very opinionated nit: can you please add extra new line after this?
Sure.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1976-1986
+  Optional KnownClassEquality =
+  isEqualTo(State, ClassOfSimplifiedSym);
+  if (KnownClassEquality) {
+// The classes are already equal, there is no need to merge.
+if (*KnownClassEquality == true)
+  continue;
+// We are about to add the newly simplified symbol to this equivalence

vsavchenko wrote:
> Now, since you put this logic into `merge`, you can just merge.
Wow, good catch.


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] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-06-09 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 350966.
martong marked an inline comment as done.
martong added a comment.

- Remove isEqualTo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103314

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/find-binop-constraints.cpp

Index: clang/test/Analysis/find-binop-constraints.cpp
===
--- /dev/null
+++ clang/test/Analysis/find-binop-constraints.cpp
@@ -0,0 +1,163 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+int test_legacy_behavior(int x, int y) {
+  if (y != 0)
+return 0;
+  if (x + y != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return y / (x + y);  // expected-warning{{Division by zero}}
+}
+
+int test_rhs_further_constrained(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (y != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_lhs_further_constrained(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (x != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_lhs_and_rhs_further_constrained(int x, int y) {
+  if (x % y != 1)
+return 0;
+  if (x != 1)
+return 0;
+  if (y != 2)
+return 0;
+  clang_analyzer_eval(x % y == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 2); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_commutativity(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (y != 0)
+return 0;
+  clang_analyzer_eval(y + x == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_binop_when_height_is_2_r(int a, int x, int y, int z) {
+  switch (a) {
+  case 1: {
+if (x + y + z != 0)
+  return 0;
+if (z != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 2: {
+if (x + y + z != 0)
+  return 0;
+if (y != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 3: {
+if (x + y + z != 0)
+  return 0;
+if (x != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 4: {
+if (x + y + z != 0)
+  return 0;
+if (x + y != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 5: {
+if (z != 0)
+  return 0;
+if (x + y + z != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+if (y != 0)
+  return 0;
+clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+break;
+  }
+
+  }
+  return 0;
+}
+
+void test_equivalence_classes_are_updated(int a, int b, int c, int d) {
+  if (a + b != c)
+return;
+  if (a != d)
+return;
+  if (b != 0)
+return;
+  clang_analyzer_eval(c == d); // expected-warning{{TRUE}}
+  // Keep the symbols and the constraints! alive.
+  (void)(a * b * c * d);
+  return;
+}
+
+void test_contradiction(int a, int b, int c, int d) {
+  if (a + b != c)
+return;
+  if (a == c)
+return;
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+
+  // Bring in the contradiction.
+  if (b != 0)
+return;
+  clang_analyzer_warnIfReached(); // no-warning, i.e. UNREACHABLE
+  // Keep the symbols and the constraints! alive.
+  (void)(a * b * c * d);
+  return;
+}
+
+void test_deferred_contradiction(int e0, int b0, int b1) {
+
+  int e1 = e0 - b0; // e1 is bound to (reg_$0) - (reg_$1)
+  (void)(b0 == 2);
+
+  int e2 = e1 - b1;
+  if (e2 > 0) { // b1 != e1
+clang_analyzer_warnIfReached();   // expected-warning{{REACHABLE}}
+// Here, e1 is still bound to (reg_$0) - (reg_$1) but we
+// should be able to simplify it to (reg_$0) - 2 and thus realize
+// the contradiction.
+if 

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

2021-06-09 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

> OK, we definitely need to know about performance.

Couldn't agree more. I am in the middle of a performance measurement that I do 
with csa-testbench (on 
memchached,tmux,curl,twin,redis,vim,openssl,sqlite,ffmpeg,postgresql,tinyxml2,libwebm,xerces,bitcoin,protobuf).
 Hopefully I can give you some results soon.

> Plus, I'm still curious about the crash. I didn't get how simplification 
> helped/caused that crash.

So, the crash was actually an assertion failure in StdLibraryFunctionsChecker, 
which came when I made a test analysis run on the twin project. The assertion 
was here:

  if (FailureSt && !SuccessSt) {
if (ExplodedNode *N = C.generateErrorNode(NewState))
  reportBug(Call, N, Constraint.get(), Summary, C);
break;
  } else {
// We will apply the constraint even if we cannot reason about the
// argument. This means both SuccessSt and FailureSt can be true. If we
// weren't applying the constraint that would mean that symbolic
// execution continues on a code whose behaviour is undefined.
assert(SuccessSt);   // 
<- This fired 
!!!
NewState = SuccessSt;
  }

With multiple creduce iterations below is a minimal example with 
StdLibraryFunctionsChecker. That crashed when we applied the `BufferSize` 
constraint of `fread`.

  typedef int FILE;
  long b;
  unsigned long fread(void *__restrict, unsigned long, unsigned long,
  FILE *__restrict);
  void foo();
  void c(int *a, int e0) {
  
int e1 = e0 - b;
b == 2;
foo();
  
int e2 = e1 - b;
if (e2 > 0 && b == e1) {
  (void)a; (void)e1; (void)c;
  fread(a, sizeof(char), e1, c);
}
  }

Turned out, the checker had the assertion because before applying the arg 
constraint and its negated counterpart, the state was already infeasible. (But 
the analyzer recognized this only when it added the new assumptions when 
checking the applicability of the arg constraint.)
Thus, I could remove `fread` and the Checker from the problem set and could 
create the test case that synthesizes the unfeasible state.


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] D103825: [clang] Do not crash when ArgTy is null in CheckArgAlignment

2021-06-09 Thread Adam Czachorowski via Phabricator via cfe-commits
adamcz added a comment.

Thanks Haojian!
I sent out https://reviews.llvm.org/D103982 for the CXXDefaultArgExpr. I'll 
update this change after I submit that one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103825

___
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

2021-06-09 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 2 inline comments as done.
martong added inline comments.



Comment at: clang/test/Analysis/find-binop-constraints.cpp:155-158
+// Here, e1 is still bound to (reg_$0) - (reg_$1) but we
+// should be able to simplify it to (reg_$0) - 2 and thus realize
+// the contradiction.
+if (b1 == e1) {

martong wrote:
> vsavchenko wrote:
> > Hmm, I don't see how simplification helped here.
> > 
> > After the previous `if` statement, we should have had two equivalence 
> > classes known to be disequal: `reg_$2` and `(reg_$0) - 
> > (reg_$1)`.
> > Further, we directly compare these two symbols.  We can figure it out 
> > without any simplifications.
> > 
> > Am I missing something here?
> When we evaluate `e2 > 0` then we will set `e1` as disequal to `b1`. However, 
> at this point because of the eager constant folding `e1` is `e0 - 2` (on the 
> path where `b0 == 2` is true). 
> 
> So, when we evaluate `b1 == e1` then this is the diseq info we have in the 
> State (I used `dumpDisEq` from D103967):
> ```
> reg_$2
> DisequalTo:
> (reg_$0) - 2
> 
> (reg_$0) - 2
> DisequalTo:
> reg_$2
> ```
> 
> And indeed we ask directly whether the LHS (`reg_$2`) is equal to 
> RHS`(reg_$0) - (reg_$1)`.  This is because the `DeclRefExpr` 
> of `e1` is still bound to SVal which originates from the time before we 
> constrained b0 to 2. With other words: the `Environment` is not changed by 
> introducing a new constraint.
> 
> BTW, this test fails even in llvm/main.
> 
> 
> 
>  With other words: the Environment is not changed by introducing a new 
> constraint.

This suggests that another approach could be to do change the `Environment` 
when we add a new constraint. I am not sure about the pros/cons atm, but might 
be worth to experiment. What do you think?


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] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:216-221
+struct test demo[] =
+{
+{56,23, "hello"},
+{-1, 93463, "world"},
+{ 7, 5,"!!"}
+};

feg208 wrote:
> curdeius wrote:
> > Don't forget to re-generate .rst.
> I am sorry. I am not entirely sure what to check here? Should I regenerate 
> the docs for the site and just ensure that they look ok?
It seems that you modified Format.h but didn't re-run 
`clang/docs/tools/dump_format_style.py` (example for `Left` is actually 
right-aligned).


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] D103982: [clang] Make CXXDefaultArgExpr inherit dependence from the inner Expr

2021-06-09 Thread Adam Czachorowski via Phabricator via cfe-commits
adamcz created this revision.
adamcz added a reviewer: hokein.
adamcz requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Before this change, CXXDefaultArgExpr would always have
ExprDependence::None. This can lead to issues when, for example, the
inner expression is RecoveryExpr and yet containsErrors() on the default
expression is false.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103982

Files:
  clang/include/clang/AST/ComputeDependence.h
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ComputeDependence.cpp
  clang/test/AST/ast-dump-default-arg-dep.cpp


Index: clang/test/AST/ast-dump-default-arg-dep.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-default-arg-dep.cpp
@@ -0,0 +1,10 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump 
-frecovery-ast %s | FileCheck %s
+
+// CXXDefaultArgExpr should inherit dependence from the inner Expr, in this 
case
+// RecoveryExpr.
+void fun(int arg = foo());
+
+void test() {
+  fun();
+}
+// CHECK: -CXXDefaultArgExpr 0x{{[^ ]*}} <> '' 
contains-errors lvalue
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -748,6 +748,10 @@
   return E->getExpr()->getDependence();
 }
 
+ExprDependence clang::computeDependence(CXXDefaultArgExpr *E) {
+  return E->getExpr()->getDependence();
+}
+
 ExprDependence clang::computeDependence(LambdaExpr *E,
 bool ContainsUnexpandedParameterPack) {
   auto D = toExprDependence(E->getType()->getDependence());
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -1257,7 +1257,7 @@
  Param->getDefaultArg()->getObjectKind()),
 Param(Param), UsedContext(UsedContext) {
 CXXDefaultArgExprBits.Loc = Loc;
-setDependence(ExprDependence::None);
+setDependence(computeDependence(this));
   }
 
 public:
Index: clang/include/clang/AST/ComputeDependence.h
===
--- clang/include/clang/AST/ComputeDependence.h
+++ clang/include/clang/AST/ComputeDependence.h
@@ -71,6 +71,7 @@
 class DependentScopeDeclRefExpr;
 class CXXConstructExpr;
 class CXXDefaultInitExpr;
+class CXXDefaultArgExpr;
 class LambdaExpr;
 class CXXUnresolvedConstructExpr;
 class CXXDependentScopeMemberExpr;
@@ -156,6 +157,7 @@
 ExprDependence computeDependence(DependentScopeDeclRefExpr *E);
 ExprDependence computeDependence(CXXConstructExpr *E);
 ExprDependence computeDependence(CXXDefaultInitExpr *E);
+ExprDependence computeDependence(CXXDefaultArgExpr *E);
 ExprDependence computeDependence(LambdaExpr *E,
  bool ContainsUnexpandedParameterPack);
 ExprDependence computeDependence(CXXUnresolvedConstructExpr *E);


Index: clang/test/AST/ast-dump-default-arg-dep.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-default-arg-dep.cpp
@@ -0,0 +1,10 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump -frecovery-ast %s | FileCheck %s
+
+// CXXDefaultArgExpr should inherit dependence from the inner Expr, in this case
+// RecoveryExpr.
+void fun(int arg = foo());
+
+void test() {
+  fun();
+}
+// CHECK: -CXXDefaultArgExpr 0x{{[^ ]*}} <> '' contains-errors lvalue
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -748,6 +748,10 @@
   return E->getExpr()->getDependence();
 }
 
+ExprDependence clang::computeDependence(CXXDefaultArgExpr *E) {
+  return E->getExpr()->getDependence();
+}
+
 ExprDependence clang::computeDependence(LambdaExpr *E,
 bool ContainsUnexpandedParameterPack) {
   auto D = toExprDependence(E->getType()->getDependence());
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -1257,7 +1257,7 @@
  Param->getDefaultArg()->getObjectKind()),
 Param(Param), UsedContext(UsedContext) {
 CXXDefaultArgExprBits.Loc = Loc;
-setDependence(ExprDependence::None);
+setDependence(computeDependence(this));
   }
 
 public:
Index: clang/include/clang/AST/ComputeDependence.h
===
--- clang/include/clang/AST/ComputeDependence.h
+++ clang/include/clang/AST/ComputeDependence.h
@@ -71,6 +71,7 @@
 class DependentScopeDeclRefExpr;
 class CXXConstructExpr;
 class CXXDefaultInitExpr;
+class CXXDefaultArgExpr;

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

2021-06-09 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone accepted this revision.
Quuxplusone added a comment.
This revision is now accepted and ready to land.

@mizvekov, my understanding is:

- D99696  was temporarily blocked on D103720 
, but now D103720 
 is landed and D99696 
 is unblocked
- D99696  is a codegen improvement in all 
language modes, and is also blocking D99005 
- D99005  is blocking our getting experience 
with p2266  for C++23

IMHO we should just plow ahead here. @aaronpuchert what's your take on D99696 
 at this point; is it good to go or still 
unresolved issues?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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


[PATCH] D103909: [CSSPGO] Emit mangled dwarf names for line tables debug option under -fpseudo-probe-for-profiling

2021-06-09 Thread Hongtao Yu 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 rG64b2fb7967a7: [CSSPGO] Emit mangled dwarf names for line 
tables debug option under -fpseudo… (authored by hoy).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103909

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-pseudo-probe.cpp


Index: clang/test/CodeGen/debug-info-pseudo-probe.cpp
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-pseudo-probe.cpp
@@ -0,0 +1,12 @@
+// This test checks if a symbol gets mangled dwarf names with 
-fpseudo-probe-for-profiling option.
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm 
-debug-info-kind=line-tables-only -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64 -x c++  -S -emit-llvm 
-debug-info-kind=line-tables-only -fpseudo-probe-for-profiling -o - < %s | 
FileCheck %s --check-prefix=MANGLE
+
+int foo() {
+  return 0;
+}
+
+// PLAIN: define dso_local i32 @_Z3foov()
+// PLAIN: distinct !DISubprogram(name: "foo", scope:
+// MANGLE: define dso_local i32 @_Z3foov()
+// MANGLE: distinct !DISubprogram(name: "foo", linkageName: "_Z3foov"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3551,6 +3551,7 @@
   if (LinkageName == Name || (!CGM.getCodeGenOpts().EmitGcovArcs &&
   !CGM.getCodeGenOpts().EmitGcovNotes &&
   !CGM.getCodeGenOpts().DebugInfoForProfiling &&
+  !CGM.getCodeGenOpts().PseudoProbeForProfiling &&
   DebugKind <= 
codegenoptions::DebugLineTablesOnly))
 LinkageName = StringRef();
 


Index: clang/test/CodeGen/debug-info-pseudo-probe.cpp
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-pseudo-probe.cpp
@@ -0,0 +1,12 @@
+// This test checks if a symbol gets mangled dwarf names with -fpseudo-probe-for-profiling option.
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm -debug-info-kind=line-tables-only -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64 -x c++  -S -emit-llvm -debug-info-kind=line-tables-only -fpseudo-probe-for-profiling -o - < %s | FileCheck %s --check-prefix=MANGLE
+
+int foo() {
+  return 0;
+}
+
+// PLAIN: define dso_local i32 @_Z3foov()
+// PLAIN: distinct !DISubprogram(name: "foo", scope:
+// MANGLE: define dso_local i32 @_Z3foov()
+// MANGLE: distinct !DISubprogram(name: "foo", linkageName: "_Z3foov"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3551,6 +3551,7 @@
   if (LinkageName == Name || (!CGM.getCodeGenOpts().EmitGcovArcs &&
   !CGM.getCodeGenOpts().EmitGcovNotes &&
   !CGM.getCodeGenOpts().DebugInfoForProfiling &&
+  !CGM.getCodeGenOpts().PseudoProbeForProfiling &&
   DebugKind <= codegenoptions::DebugLineTablesOnly))
 LinkageName = StringRef();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 64b2fb7 - [CSSPGO] Emit mangled dwarf names for line tables debug option under -fpseudo-probe-for-profiling

2021-06-09 Thread Hongtao Yu via cfe-commits

Author: Hongtao Yu
Date: 2021-06-09T10:46:03-07:00
New Revision: 64b2fb7967a749b83f59656f0cd2f4d00501efaa

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

LOG: [CSSPGO] Emit mangled dwarf names for line tables debug option under 
-fpseudo-probe-for-profiling

Reviewed By: wenlei

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

Added: 
clang/test/CodeGen/debug-info-pseudo-probe.cpp

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 1367ef46d85d..080d494a2830 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3551,6 +3551,7 @@ void CGDebugInfo::collectFunctionDeclProps(GlobalDecl GD, 
llvm::DIFile *Unit,
   if (LinkageName == Name || (!CGM.getCodeGenOpts().EmitGcovArcs &&
   !CGM.getCodeGenOpts().EmitGcovNotes &&
   !CGM.getCodeGenOpts().DebugInfoForProfiling &&
+  !CGM.getCodeGenOpts().PseudoProbeForProfiling &&
   DebugKind <= 
codegenoptions::DebugLineTablesOnly))
 LinkageName = StringRef();
 

diff  --git a/clang/test/CodeGen/debug-info-pseudo-probe.cpp 
b/clang/test/CodeGen/debug-info-pseudo-probe.cpp
new file mode 100644
index ..78a684cd1f39
--- /dev/null
+++ b/clang/test/CodeGen/debug-info-pseudo-probe.cpp
@@ -0,0 +1,12 @@
+// This test checks if a symbol gets mangled dwarf names with 
-fpseudo-probe-for-profiling option.
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm 
-debug-info-kind=line-tables-only -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64 -x c++  -S -emit-llvm 
-debug-info-kind=line-tables-only -fpseudo-probe-for-profiling -o - < %s | 
FileCheck %s --check-prefix=MANGLE
+
+int foo() {
+  return 0;
+}
+
+// PLAIN: define dso_local i32 @_Z3foov()
+// PLAIN: distinct !DISubprogram(name: "foo", scope:
+// MANGLE: define dso_local i32 @_Z3foov()
+// MANGLE: distinct !DISubprogram(name: "foo", linkageName: "_Z3foov"



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


[PATCH] D103845: [compiler-rt][hwasan] Add newline between record_addr lines on frame record dumps

2021-06-09 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment.

Thanks for tracking this down. It seems best to change the Printf call to add 
the newline, since otherwise it looks like you'd be adding a spurious newline 
to the other callers of `RenderFrame` on Fuchsia.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103845

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


[PATCH] D103894: NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

2021-06-09 Thread Stephen Concannon via Phabricator via cfe-commits
Stephen updated this revision to Diff 350950.
Stephen added a comment.

Fix the expected error messages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103894

Files:
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to signed type 'int' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to signed type 'float' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to signed type 'long long' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to signed type 'double' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to signed type 'float' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to signed type 'long long' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'double' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 8908903 - Corrects some minor issues with the CXX status page.

2021-06-09 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2021-06-09T13:38:41-04:00
New Revision: 8908903eacb0e0a79c004e49959f860b5c0e4d53

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

LOG: Corrects some minor issues with the CXX status page.

1) Adds some  tags where they were missing.
2) Documents that C++14 is the current default language mode, not C++98

Added: 


Modified: 
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index fc94282311561..1b761be8b22ae 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -90,12 +90,11 @@ C++11 implementation status
 
 Clang 3.3 and later implement all of the https://www.iso.org/standard/50372.html;>ISO
-  C++ 2011 standard.
+  C++ 2011 standard.
 
-By default, Clang builds C++ code according to the C++98 standard, with many
-C++11 features accepted as extensions. You can use Clang in C++11 mode with the
--std=c++11 option. Clang's C++11 mode can be used
-with https://libcxx.llvm.org/;>libc++ or with gcc's libstdc++.
+You can use Clang in C++11 mode with the -std=c++11
+option. Clang's C++11 mode can be used with
+https://libcxx.llvm.org/;>libc++ or with gcc's libstdc++.
 
 
 List of features and minimum Clang version with support
@@ -497,9 +496,10 @@ C++14 implementation status
 
 Clang 3.4 and later implement all of the https://www.iso.org/standard/64029.html;>ISO
-C++ 2014 standard.
+C++ 2014 standard.
 
-You can use Clang in C++14 mode with the -std=c++14 option
+By default, Clang builds C++ code according to the C++14 standard.
+You can use Clang in C++14 mode with the -std=c++14 option
 (use -std=c++1y in Clang 3.4 and earlier).
 
 
@@ -591,7 +591,7 @@ C++14 implementation status
 C++17 implementation status
 
 Clang 5 and later implement all the features of the
-https://www.iso.org/standard/68564.html;>ISO C++ 2017 standard.
+https://www.iso.org/standard/68564.html;>ISO C++ 2017 
standard.
 
 You can use Clang in C++17 mode with the -std=c++17 option
 (use -std=c++1z in Clang 4 and earlier).
@@ -856,7 +856,7 @@ C++17 implementation status
 C++20 implementation status
 
 Clang has support for some of the features of the
-https://www.iso.org/standard/79358.html;>ISO C++ 2020 standard.
+https://www.iso.org/standard/79358.html;>ISO C++ 2020 
standard.
 
 You can use Clang in C++20 mode with the -std=c++20 option
 (use -std=c++2a in Clang 9 and earlier).
@@ -1253,7 +1253,7 @@ C++20 implementation status
 C++2b implementation status
 
 Clang has support for some of the features of the C++ standard following
-C++20, informally referred to as C++2b.
+C++20, informally referred to as C++2b.
 
 You can use Clang in C++2b mode with the -std=c++2b option.
 



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


[PATCH] D101139: Create install targets for scan-build-py.

2021-06-09 Thread Petr Hosek via Phabricator via cfe-commits
phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D101139

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


[PATCH] D103967: [Analyzer][solver] Add dump methods for (dis)equality classes.

2021-06-09 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

This is definitely useful!  Thanks!
I was just wondering if we should add it into the state printer instead.  @NoQ 
what's your take on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103967

___
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

2021-06-09 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 2 inline comments as done.
martong added a comment.

> I have one thought here. If the lack of simplification indeed caused the 
> crash, we are in trouble with this patch. IMO simplification in just one 
> place should make it better, but shouldn't produce infeasible states for us. 
> In other words, any number simplifications is a conservative operation that 
> makes our lives a bit better. The moment they become a requirement (i.e. 
> simplifications call for more simplifications or we crash) this solution from 
> this patch has to become much harder. This is because whenever we do merge, 
> we essentially can create another situation when we find out that some 
> symbolic expression is a constant. Let's say that we are merging classes A 
> and B which have constraints [INT_MIN, 42] and [42, INT_MAX]. After the 
> merge, we are positive that all the members of this new class are equal to 
> 42. And if so, we can further simplify classes and their members. This 
> algorithm turns into a fixed point algorithm, which has a good chance to 
> sabotage our performance.

Yes, good point(s). I am trying to avoid turning into a fixed point algorithm 
by directly iterating over the equivalence classes instead of reusing the 
existing track mechanism. On the other hand, perhaps with some budge the 
fixpoint algo would be worth to experiment with.




Comment at: clang/test/Analysis/find-binop-constraints.cpp:150
+  int e1 = e0 - b0; // e1 is bound to (reg_$0) - (reg_$1)
+  (void)(b0 == 2);
+

vsavchenko wrote:
> It's not really connected to your patch, but this confuses me!  Why does the 
> analyzer think that `b0` is guaranteed to be 2 after this statement.  Even if 
> we eagerly assume here, shouldn't it mean that there are still two paths `b0 
> == 2` and `b0 != 2`?
Don't be puzzled by this. This indeed bifurcates. The interesting path is where 
`b0 == 2` is true.

I am going to update this line with `if (b0 ==2) {` to achieve a similar 
effect. (I was using creduce and tried to simplify even more after that, but i 
missed this.)



Comment at: clang/test/Analysis/find-binop-constraints.cpp:155-158
+// Here, e1 is still bound to (reg_$0) - (reg_$1) but we
+// should be able to simplify it to (reg_$0) - 2 and thus realize
+// the contradiction.
+if (b1 == e1) {

vsavchenko wrote:
> Hmm, I don't see how simplification helped here.
> 
> After the previous `if` statement, we should have had two equivalence classes 
> known to be disequal: `reg_$2` and `(reg_$0) - (reg_$1 b0>)`.
> Further, we directly compare these two symbols.  We can figure it out without 
> any simplifications.
> 
> Am I missing something here?
When we evaluate `e2 > 0` then we will set `e1` as disequal to `b1`. However, 
at this point because of the eager constant folding `e1` is `e0 - 2` (on the 
path where `b0 == 2` is true). 

So, when we evaluate `b1 == e1` then this is the diseq info we have in the 
State (I used `dumpDisEq` from D103967):
```
reg_$2
DisequalTo:
(reg_$0) - 2

(reg_$0) - 2
DisequalTo:
reg_$2
```

And indeed we ask directly whether the LHS (`reg_$2`) is equal to 
RHS`(reg_$0) - (reg_$1)`.  This is because the `DeclRefExpr` of 
`e1` is still bound to SVal which originates from the time before we 
constrained b0 to 2. With other words: the `Environment` is not changed by 
introducing a new constraint.

BTW, this test fails even in llvm/main.





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] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 350924.
feg208 added a comment.

Ick. Missed a semi colon in the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"}, // first line\n"
+   "{-1, 93463, \"world\"}, // second line\n"
+   "{ 7, 5,\"!!\"}  // third line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+   "{ 56,23, 21,   \"oh\"}, // first line\n"
+   "{ -1, 93463, 22,   \"my\"}, // second line\n"
+   "{  7, 5,  1, \"goodness\"}  // third line\n"
+   "{234, 5,  1, \"gracious\"}  // fourth line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{int{56},23, \"hello\"},\n"
+   "{int{-1}, 93463, \"world\"},\n"
+   "{ int{7}, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("demo = std::array{\n"
+   "test{56,23, \"hello\"},\n"
+   "test{-1, 93463, \"world\"},\n"
+   "test{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "#if X\n"
+   "{-1, 93463, \"world\"},\n"
+   "#endif\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat(
+  "test demo[] = {\n"
+  "{ 7,23,\n"
+  " \"hello world i am a very long line that really, in any\"\n"
+  " \"just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  \"world\"},\n"
+  "{56, 5, \"!!\"}\n"
+  "};\n",
+  Style);
+
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+   "{{\"sign\"}, \"Sign\",  "
+   "  {\"x\", \"dy\"}},\n"
+   "{  {\"dx\"},  \"Mul\", {\"dy\""
+   ", \"sign\"}},\n"
+   "});\n",
+   Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+  "test demo[] = {\n"
+  "{56,23, \"hello world i am a very long line that really, "
+  "in any just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  "
+  " \"world\"},\n"
+  "{ 7, 5,  "
+  "\"!!\"},\n"
+  "};",
+  format("test demo[] = {{56, 23, \"hello world i am a very long line "
+ 

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 350920.
feg208 added a comment.

Fixes the busted test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"}, // first line\n"
+   "{-1, 93463, \"world\"}, // second line\n"
+   "{ 7, 5,\"!!\"}  // third line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+   "{ 56,23, 21,   \"oh\"}, // first line\n"
+   "{ -1, 93463, 22,   \"my\"}, // second line\n"
+   "{  7, 5,  1, \"goodness\"}  // third line\n"
+   "{234, 5,  1, \"gracious\"}  // fourth line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{int{56},23, \"hello\"},\n"
+   "{int{-1}, 93463, \"world\"},\n"
+   "{ int{7}, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("demo = std::array{\n"
+   "test{56,23, \"hello\"},\n"
+   "test{-1, 93463, \"world\"},\n"
+   "test{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "#if X\n"
+   "{-1, 93463, \"world\"},\n"
+   "#endif\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat(
+  "test demo[] = {\n"
+  "{ 7,23,\n"
+  " \"hello world i am a very long line that really, in any\"\n"
+  " \"just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  \"world\"},\n"
+  "{56, 5, \"!!\"}\n"
+  "};\n",
+  Style);
+
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+   "{{\"sign\"}, \"Sign\",  "
+   "  {\"x\", \"dy\"}},\n"
+   "{  {\"dx\"},  \"Mul\", {\"dy\""
+   ", \"sign\"}},\n"
+   "});\n",
+   Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+  "test demo[] = {\n"
+  "{56,23, \"hello world i am a very long line that really, "
+  "in any just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  "
+  " \"world\"},\n"
+  "{ 7, 5,  "
+  "\"!!\"},\n"
+  "};",
+  format("test demo[] = {{56, 23, \"hello world i am a very long line "
+ "that really, 

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

2021-06-09 Thread Michael Benfield via Phabricator via cfe-commits
mbenfield added inline comments.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:7794-7798
+if (BO->getLHS()->getType()->isDependentType() ||
+BO->getRHS()->getType()->isDependentType())
+{
+  if (BO->getOpcode() != BO_Assign)
+return;

Would you mind elaborating on the need for this code? IIUC, you're concerned 
about overloaded operators, but won't such operators always be covered by the 
`CXXOperatorCallExpr` case below? 


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

https://reviews.llvm.org/D103949

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


[PATCH] D103934: clang/darwin: use response files with ld64

2021-06-09 Thread Keith Smiley 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 rG1c7f3395b8ec: clang/darwin: use response files with ld64 
(authored by keith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103934

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -711,10 +711,7 @@
   }
 
   ResponseFileSupport ResponseSupport;
-  if (LinkerIsLLDDarwinNew) {
-// Xcode12's ld64 added support for @response files, but it's crashy:
-// https://openradar.appspot.com/radar?id=4933317065441280
-// FIXME: Pass this for ld64 once it no longer crashes.
+  if (Version[0] >= 705 || LinkerIsLLDDarwinNew) {
 ResponseSupport = ResponseFileSupport::AtFileUTF8();
   } else {
 // For older versions of the linker, use the legacy filelist method 
instead.


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -711,10 +711,7 @@
   }
 
   ResponseFileSupport ResponseSupport;
-  if (LinkerIsLLDDarwinNew) {
-// Xcode12's ld64 added support for @response files, but it's crashy:
-// https://openradar.appspot.com/radar?id=4933317065441280
-// FIXME: Pass this for ld64 once it no longer crashes.
+  if (Version[0] >= 705 || LinkerIsLLDDarwinNew) {
 ResponseSupport = ResponseFileSupport::AtFileUTF8();
   } else {
 // For older versions of the linker, use the legacy filelist method instead.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 1c7f339 - clang/darwin: use response files with ld64

2021-06-09 Thread Keith Smiley via cfe-commits

Author: Keith Smiley
Date: 2021-06-09T09:04:37-07:00
New Revision: 1c7f3395b8ec52462220898495883ec570390367

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

LOG: clang/darwin: use response files with ld64

This crasher was fixed with Xcode 13.0 beta 1 / ld64 705. This is an
updated revert of https://reviews.llvm.org/D92357

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Darwin.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index 5ccd2300afd39..8a0c4721eea0e 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -711,10 +711,7 @@ void darwin::Linker::ConstructJob(Compilation , const 
JobAction ,
   }
 
   ResponseFileSupport ResponseSupport;
-  if (LinkerIsLLDDarwinNew) {
-// Xcode12's ld64 added support for @response files, but it's crashy:
-// https://openradar.appspot.com/radar?id=4933317065441280
-// FIXME: Pass this for ld64 once it no longer crashes.
+  if (Version[0] >= 705 || LinkerIsLLDDarwinNew) {
 ResponseSupport = ResponseFileSupport::AtFileUTF8();
   } else {
 // For older versions of the linker, use the legacy filelist method 
instead.



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


[PATCH] D103495: [static initializers] Emit global_ctors and global_dtors in reverse order when init_array is not used.

2021-06-09 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: probinson.
rnk added a comment.

In D103495#2806097 , @jyknight wrote:

> Won't this change cause weird effects with LTO, when you're merging multiple 
> TUs' global_ctors arrays before emitting? Won't this end up reversing the 
> order of the files, as well as the order of the functions within a single 
> file? That does not seem likely to be expected, right?

True, I think that is an issue, and that probably does affect Sony's actual 
customers. I hear games tend to use regular LTO, so this will in fact reverse 
the order of initialization between object files for those users. The only 
solution I can think of would be to bring the LTO library into the game. It 
must know whether .ctors of .init_array are in use, since it generates code and 
it has to pick the section. To ensure that initialization of objects happens in 
reverse command line order as expected of users of -fno-init-array, the linker 
would join global_ctors in reverse order. That's not elegant. It encodes this 
knowledge in two places instead of one. But it does move in the direction of 
making things more defined, more deterministic, and that seems like a good 
thing.

So, a question for @wolfgangp and @probinson , do we need to make adjustments 
to the LTO library, or is this OK as is?

I think for non-Sony users we can accept the change in initialization order.


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

https://reviews.llvm.org/D103495

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


[PATCH] D99675: [llvm][clang] Create new intrinsic llvm.arithmetic.fence to control FP optimization at expression level

2021-06-09 Thread Melanie Blower via Phabricator via cfe-commits
mibintc updated this revision to Diff 350912.
mibintc retitled this revision from "[llvm][clang] Create new intrinsic 
llvm.arith.fence to control FP optimization at expression level" to 
"[llvm][clang] Create new intrinsic llvm.arithmetic.fence to control FP 
optimization at expression level".
mibintc added a comment.

Correct small formatting issue in LangRef.rst thanks @pengfei


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99675

Files:
  llvm/docs/LangRef.rst
  llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
  llvm/include/llvm/CodeGen/BasicTTIImpl.h
  llvm/include/llvm/CodeGen/ISDOpcodes.h
  llvm/include/llvm/CodeGen/SelectionDAGISel.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/Support/TargetOpcodes.def
  llvm/include/llvm/Target/Target.td
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
  llvm/test/CodeGen/X86/arithmetic_fence.ll
  llvm/test/CodeGen/X86/arithmetic_fence2.ll

Index: llvm/test/CodeGen/X86/arithmetic_fence2.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/arithmetic_fence2.ll
@@ -0,0 +1,170 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=i686-unknown-unknown -mattr=+sse2 | FileCheck %s --check-prefix=X86
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+sse2 | FileCheck %s --check-prefix=X64
+
+define double @f1(double %a) {
+; X86-LABEL: f1:
+; X86:   # %bb.0:
+; X86-NEXT:pushl %ebp
+; X86-NEXT:.cfi_def_cfa_offset 8
+; X86-NEXT:.cfi_offset %ebp, -8
+; X86-NEXT:movl %esp, %ebp
+; X86-NEXT:.cfi_def_cfa_register %ebp
+; X86-NEXT:andl $-8, %esp
+; X86-NEXT:subl $8, %esp
+; X86-NEXT:movsd {{.*#+}} xmm0 = mem[0],zero
+; X86-NEXT:mulsd {{\.?LCPI[0-9]+_[0-9]+}}, %xmm0
+; X86-NEXT:movsd %xmm0, (%esp)
+; X86-NEXT:fldl (%esp)
+; X86-NEXT:movl %ebp, %esp
+; X86-NEXT:popl %ebp
+; X86-NEXT:.cfi_def_cfa %esp, 4
+; X86-NEXT:retl
+;
+; X64-LABEL: f1:
+; X64:   # %bb.0:
+; X64-NEXT:mulsd {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; X64-NEXT:retq
+  %1 = fadd fast double %a, %a
+  %2 = fadd fast double %a, %a
+  %3 = fadd fast double %1, %2
+  ret double %3
+}
+
+define double @f2(double %a) {
+; X86-LABEL: f2:
+; X86:   # %bb.0:
+; X86-NEXT:pushl %ebp
+; X86-NEXT:.cfi_def_cfa_offset 8
+; X86-NEXT:.cfi_offset %ebp, -8
+; X86-NEXT:movl %esp, %ebp
+; X86-NEXT:.cfi_def_cfa_register %ebp
+; X86-NEXT:andl $-8, %esp
+; X86-NEXT:subl $8, %esp
+; X86-NEXT:movsd {{.*#+}} xmm0 = mem[0],zero
+; X86-NEXT:addsd %xmm0, %xmm0
+; X86-NEXT:movapd %xmm0, %xmm1
+; X86-NEXT:#ARITH_FENCE
+; X86-NEXT:addsd %xmm0, %xmm1
+; X86-NEXT:movsd %xmm1, (%esp)
+; X86-NEXT:fldl (%esp)
+; X86-NEXT:movl %ebp, %esp
+; X86-NEXT:popl %ebp
+; X86-NEXT:.cfi_def_cfa %esp, 4
+; X86-NEXT:retl
+;
+; X64-LABEL: f2:
+; X64:   # %bb.0:
+; X64-NEXT:addsd %xmm0, %xmm0
+; X64-NEXT:movapd %xmm0, %xmm1
+; X64-NEXT:#ARITH_FENCE
+; X64-NEXT:addsd %xmm0, %xmm1
+; X64-NEXT:movapd %xmm1, %xmm0
+; X64-NEXT:retq
+  %1 = fadd fast double %a, %a
+  %t = call double @llvm.arithmetic.fence.f64(double %1)
+  %2 = fadd fast double %a, %a
+  %3 = fadd fast double %t, %2
+  ret double %3
+}
+
+define <2 x float> @f3(<2 x float> %a) {
+; X86-LABEL: f3:
+; X86:   # %bb.0:
+; X86-NEXT:mulps {{\.?LCPI[0-9]+_[0-9]+}}, %xmm0
+; X86-NEXT:retl
+;
+; X64-LABEL: f3:
+; X64:   # %bb.0:
+; X64-NEXT:mulps {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; X64-NEXT:retq
+  %1 = fadd fast <2 x float> %a, %a
+  %2 = fadd fast <2 x float> %a, %a
+  %3 = fadd fast <2 x float> %1, %2
+  ret <2 x float> %3
+}
+
+define <2 x float> @f4(<2 x float> %a) {
+; X86-LABEL: f4:
+; X86:   # %bb.0:
+; X86-NEXT:addps %xmm0, %xmm0
+; X86-NEXT:movaps %xmm0, %xmm1
+; X86-NEXT:#ARITH_FENCE
+; X86-NEXT:addps %xmm0, %xmm1
+; X86-NEXT:movaps %xmm1, %xmm0
+; X86-NEXT:retl
+;
+; X64-LABEL: f4:
+; X64:   # %bb.0:
+; X64-NEXT:addps %xmm0, %xmm0
+; X64-NEXT:movaps %xmm0, %xmm1
+; X64-NEXT:#ARITH_FENCE
+; X64-NEXT:addps %xmm0, %xmm1
+; X64-NEXT:movaps %xmm1, %xmm0
+; X64-NEXT:retq
+  %1 = fadd fast <2 x float> %a, %a
+  %t = call <2 x float> @llvm.arithmetic.fence.v2f32(<2 x float> %1)
+  %2 = fadd fast <2 x float> %a, %a
+  %3 = fadd fast <2 x float> %t, %2
+  ret <2 x float> %3
+}
+
+define <8 x float> @f5(<8 x float> %a) {
+; X86-LABEL: f5:
+; X86:   # %bb.0:
+; X86-NEXT:movaps {{.*#+}} xmm2 = [4.0E+0,4.0E+0,4.0E+0,4.0E+0]
+; X86-NEXT:mulps %xmm2, %xmm0
+; X86-NEXT:mulps %xmm2, %xmm1
+; X86-NEXT:retl
+;
+; X64-LABEL: f5:
+; 

  1   2   >