[clang] fe611b1 - [clang] Move the soname declaration in a variable at the top of the file

[Sylvestre Ledru via cfe-commits]

Author: Sylvestre Ledru
Date: 2021-08-27T09:07:12+02:00
New Revision: fe611b1da84b9442c093739394d336af9e99c1a1

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

LOG: [clang] Move the soname declaration in a variable at the top of the file

Currently, it is a bit buried in the file even if this is
pretty important for distro.

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/tools/libclang/CMakeLists.txt

Removed: 




diff  --git a/clang/tools/libclang/CMakeLists.txt 
b/clang/tools/libclang/CMakeLists.txt
index 7148bdccfba48..bf88dca0a34b1 100644
--- a/clang/tools/libclang/CMakeLists.txt
+++ b/clang/tools/libclang/CMakeLists.txt
@@ -1,3 +1,9 @@
+# The SOVERSION should be updated only if a change is made to the libclang
+# ABI, and when it is updated, it should be updated to the current
+# LLVM_VERSION_MAJOR.
+# Please also see clang/tools/libclang/libclang.map
+set(CLANG_SONAME 13)
+
 set(SOURCES
   ARCMigrate.cpp
   BuildSystem.cpp
@@ -171,12 +177,9 @@ if(ENABLE_SHARED)
 set_property(SOURCE ARCMigrate.cpp APPEND PROPERTY
  OBJECT_DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/libclang.map)
 
-# The SOVERSION should be updated only if a change is made to the libclang
-# ABI, and when it is updated, it should be updated to the current
-# LLVM_VERSION_MAJOR.
 set_target_properties(libclang PROPERTIES
   VERSION 
${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}${LLVM_VERSION_SUFFIX}
-  SOVERSION 13)
+  SOVERSION ${CLANG_SONAME})
   endif()
 endif()
 



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

[PATCH] D108533: [clang] Move the soname declaration in a variable at the top of the file

[Sylvestre Ledru via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfe611b1da84b: [clang] Move the soname declaration in a 
variable at the top of the file (authored by sylvestre.ledru).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108533

Files:
  clang/tools/libclang/CMakeLists.txt


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -1,3 +1,9 @@
+# The SOVERSION should be updated only if a change is made to the libclang
+# ABI, and when it is updated, it should be updated to the current
+# LLVM_VERSION_MAJOR.
+# Please also see clang/tools/libclang/libclang.map
+set(CLANG_SONAME 13)
+
 set(SOURCES
   ARCMigrate.cpp
   BuildSystem.cpp
@@ -171,12 +177,9 @@
 set_property(SOURCE ARCMigrate.cpp APPEND PROPERTY
  OBJECT_DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/libclang.map)
 
-# The SOVERSION should be updated only if a change is made to the libclang
-# ABI, and when it is updated, it should be updated to the current
-# LLVM_VERSION_MAJOR.
 set_target_properties(libclang PROPERTIES
   VERSION 
${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}${LLVM_VERSION_SUFFIX}
-  SOVERSION 13)
+  SOVERSION ${CLANG_SONAME})
   endif()
 endif()
 


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -1,3 +1,9 @@
+# The SOVERSION should be updated only if a change is made to the libclang
+# ABI, and when it is updated, it should be updated to the current
+# LLVM_VERSION_MAJOR.
+# Please also see clang/tools/libclang/libclang.map
+set(CLANG_SONAME 13)
+
 set(SOURCES
   ARCMigrate.cpp
   BuildSystem.cpp
@@ -171,12 +177,9 @@
 set_property(SOURCE ARCMigrate.cpp APPEND PROPERTY
  OBJECT_DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/libclang.map)
 
-# The SOVERSION should be updated only if a change is made to the libclang
-# ABI, and when it is updated, it should be updated to the current
-# LLVM_VERSION_MAJOR.
 set_target_properties(libclang PROPERTIES
   VERSION ${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}${LLVM_VERSION_SUFFIX}
-  SOVERSION 13)
+  SOVERSION ${CLANG_SONAME})
   endif()
 endif()
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108618: [CGCall] Add NoInline attr if presented for the target

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro abandoned this revision.
djtodoro added a comment.

In D108618#2968626 , @rjmccall wrote:

> Does LLVM model `noinline` as a call-site attribute in the way that would be 
> necessary to get that effect?  Also, are you actually having a problem here, 
> or is this just something you noticed in the code?

It is being ignored completely, anyway :/ I am not having a problem, actually. 
It has been noticed accidentally when looking at some other attribute, but then 
I've started playing with some Debug Info cases (since that is the area I 
mostly work) by dancing around with some cross CU referencing during when using 
LTO -- all in all, this isn't necessary.

> I'm not sure we can warn about putting the attribute on a declaration; it's 
> presumably still picked up by later definitions.  There's probably a warning 
> if they conflict, as they would if the first declaration was in a header 
> included in both places, which is best practice.

Oh yes... It is very hard to warn during compilation since we don't have the 
definition -- even though we have conflicts in `test-1.c` and `test-2.c`.

@rjmccall Thanks for your comments anyway :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108618

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I'm good with words like `List of Strings` but I don't think we need `Enum`

`unsigned`  I think Integer, I'm not sure what the code is even going to do if 
you supply a -ve, Warn I hope! (there's contribution idea number 2 for you 
right there!) ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1984
+auto checkTypeIsTypedef = [](auto &&checkTypeIsTypedef,
+ const auto Ty) -> bool {
+  bool isTypedef = false;

Hah.  The knot-tying here is really cute, but I think either just make it a 
static function somewhere in the file, or make it iterative instead of 
recursive.

...actually, I think it would be better to just make the computation in 
`getTypeInfo` preserve more information.  Make `TypeInfo` carry a three-valued 
enum instead of an `AlignIsRequired` flag:

```
enum class AlignRequirement {
  /// The alignment was not explicit in code.
  None,

  /// The alignment comes from an alignment attribute on a typedef.
  RequiredByTypedef,

  /// The alignment comes from an alignment attribute on a record type.
  RequiredByRecord,
};
```

You can add a method on `TypeInfo` to ask if the alignment is required for any 
reason and make all the existing clients use that instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

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

[PATCH] D108618: [CGCall] Add NoInline attr if presented for the target

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Alright, no worries.  It was reasonable to ask if this was something we should 
fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108618

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

[PATCH] D108792: [M68k] Update pointer data layout

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

@ricky26 Could you also get this fix backported in the LLVM fork of Rust?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108792

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

[PATCH] D108808: [clang-tidy] bugprone-infinite-loop: Fix false positives with volatile addresses.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: alexfh, gribozavr2, aaron.ballman, xazax.hun.
Herald added subscribers: martong, mgehre, rnkovacs.
NoQ requested review of this revision.
Herald added a project: clang-tools-extra.

Low-level code may occasionally deal with direct access by concrete addresses 
such as `0x1234`. Values at these addresses act like globals: they can change 
at any time. They typically wear volatile qualifiers.

Suppress all warnings on loops with conditions that involve casting anything to 
a pointer-to-...-pointer-to-volatile type.

We should probably suppress loops that dereference concrete addresses 
regardless of volatile qualifiers but it's pretty difficult to figure out 
whether the address is indeed concrete (say, it may have an unknown offset, eg. 
`(char *)(0x1234 + i)`, and now it can be interpreted as either `((char 
*)0x1234)[i]` or  `((char *)i)[0x1234]` so it's unclear whether this should be 
suppressed). I also suspect that such code's behavior is undefined so this 
isn't a very important case to support. So for now i'm sticking to supporting 
the explicitly volatile-qualified case which seems straightforward.

The closely related `bugprone-redundant-branch-condition` check doesn't seem to 
be affected. I added a test just in case.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D108808

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -1387,3 +1387,13 @@
 }
   }
 }
+
+void volatile_concrete_address() {
+  // No warning. The value behind the volatile concrete address
+  // is beyond our control. It may change at any time.
+  if (*(volatile int *)0x1234) {
+if (*(volatile int *)0x1234) {
+  doSomething();
+}
+  }
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -591,3 +591,35 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none 
of its condition variables (x) are updated in the loop body 
[bugprone-infinite-loop]
   }
 }
+
+void test_volatile_cast() {
+  // This is a no-op cast. Clang ignores the qualifier, we should too.
+  for (int i = 0; (volatile int)i < 10;) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of 
its condition variables (i) are updated in the loop body 
[bugprone-infinite-loop]
+  }
+}
+
+void test_volatile_concrete_address(int i, int size) {
+  // No warning. The value behind the volatile concrete address
+  // is beyond our control. It may change at any time.
+  for (; *((volatile int *)0x1234) < size;) {
+  }
+
+  for (; *((volatile int *)(0x1234 + i)) < size;) {
+  }
+
+  for (; **((volatile int **)0x1234) < size;) {
+  }
+
+  volatile int *x = (volatile int *)0x1234;
+  for (; *x < 10;) {
+  }
+
+  // FIXME: This one should probably also be suppressed.
+  // Whatever the developer is doing here, they can do that again anywhere else
+  // which basically makes it a global.
+  for (; *(int *)0x1234 < size;) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of 
its condition variables (size) are updated in the loop body 
[bugprone-infinite-loop]
+  }
+}
+
Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -65,6 +65,17 @@
  ObjCIvarRefExpr, ObjCPropertyRefExpr, ObjCMessageExpr>(Cond)) 
{
 // FIXME: Handle MemberExpr.
 return true;
+  } else if (const auto *CE = dyn_cast(Cond)) {
+QualType T = CE->getType();
+while (true) {
+  if (T.isVolatileQualified())
+return true;
+
+  if (!T->isAnyPointerType() && !T->isReferenceType())
+break;
+
+  T = T->getPointeeType();
+}
   }
 
   return false;


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -1387,3 +1387,13 @@
 }
   }
 }
+
+void volatile_concrete_address() {
+  // No 

[PATCH] D107450: [clang-tidy] Fix wrong FIxIt in performance-move-const-arg

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

I'm a bit confused about this, but it has been a while since I read this patch. 
Am I right to think that the code in the patch and the original submission (the 
patch summary) diverged since the review started? I do not see anything 
"removed" from the test files, only a new addition. Or did you accidentally 
upload a wrong diff somewhere? The original intention of the patch was to 
remove bogus printouts.

In general: the test file ends without any testing for functions with multiple 
parameters. Are they out-of-scope by design?




Comment at: 
clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:131-145
 auto Diag = diag(FileMoveRange.getBegin(),
  "std::move of the %select{|const }0"
- "%select{expression|variable %4}1 "
- "%select{|of the trivially-copyable type %5 }2"
- "has no effect; remove std::move()"
- "%select{| or make the variable non-const}3")
+ "%select{expression|variable %5}1 "
+ "%select{|of the trivially-copyable type %6 }2"
+ "has no effect%select{; remove std::move()||; consider "
+ "changing %7's parameter from %8 to 'const %9 &'}3"
+ "%select{| or make the variable non-const}4")





Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:139
 << IsConstArg << IsVariable << IsTriviallyCopyable
-<< (IsConstArg && IsVariable && !IsTriviallyCopyable) << Var
-<< Arg->getType();
+<< (IsRValueReferenceArg + (IsRValueReferenceArg &&
+IsTriviallyCopyable &&

**`+`**? Shouldn't this be `||`? I know they're equivalent over Z_2, but if 
we're using `&&` instead of `*` then let's adhere to the style.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:262-263
+  int a = 10;
+  forwardToShowInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' 
of the trivially-copyable type 'int' has no effect
+}

Quuxplusone wrote:
> Sockke wrote:
> > Quuxplusone wrote:
> > > ```
> > >   forwardToShowInt(std::move(a));
> > >   // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 
> > > 'a' of the trivially-copyable type 'int' has no effect
> > > ```
> > > I continue to want-not-to-block this PR, since I think it's improving the 
> > > situation. However, FWIW...
> > > It's good that this message doesn't contain a fixit, since there's 
> > > nothing the programmer can really do to "fix" this call. But then, should 
> > > this message be emitted at all? If this were `clang -Wall`, then we 
> > > definitely would //not// want to emit a "noisy" warning where there's 
> > > basically nothing the programmer can do about it. Does clang-tidy have a 
> > > similar philosophy? Or is it okay for clang-tidy to say "This code looks 
> > > wonky" even when there's no obvious way to fix it?
> > > ```
> > >   forwardToShowInt(std::move(a));
> > >   // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 
> > > 'a' of the trivially-copyable type 'int' has no effect
> > > ```
> > > I continue to want-not-to-block this PR, since I think it's improving the 
> > > situation. However, FWIW...
> > > It's good that this message doesn't contain a fixit, since there's 
> > > nothing the programmer can really do to "fix" this call. But then, should 
> > > this message be emitted at all? If this were `clang -Wall`, then we 
> > > definitely would //not// want to emit a "noisy" warning where there's 
> > > basically nothing the programmer can do about it. Does clang-tidy have a 
> > > similar philosophy? Or is it okay for clang-tidy to say "This code looks 
> > > wonky" even when there's no obvious way to fix it?
> > 
> > Yes, I agree with you. I did not remove warning to maintain the original 
> > behavior, which will make the current patch clearer. In any case, it is 
> > easy for me to make this modification if you want. 
> I'll defer on this subject to whomever you get to review the code change in 
> `MoveConstArgCheck.cpp`. (@whisperity perhaps?)
(Negative, it would be @aaron.ballman or @alexfh or the other maintainers. I'm 
just trying to give back to the community after taking away //a lot// of 
reviewer effort with a humongous check that I just recently pushed in after 
//years// of (re-)development.)



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:262-263
+  int a = 10;
+  forwardToShowInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' 
of the trivially-copyable type 'int' has no effect
+}

whisperity wrote:
> Quuxplusone wrote:
> > Sockke wr

[PATCH] D108808: [clang-tidy] bugprone-infinite-loop: Fix false positives with volatile addresses.

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

What happens for something like the following? Maybe it's also worth a test 
case?

  bool wait_analogue_pin(volatile int* address, int threshold) {
while (*address < threshold) {}
return true;
  }


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D108808

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

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Possible bug https://bugs.llvm.org/show_bug.cgi?id=51640




Comment at: clang/include/clang/Format/Format.h:547
   ///   false:
-  ///   enum
-  ///   {
+  ///   enum {
   /// A,

Post commit Nit: @lunasorcery 

Rule of thumb in clang-format dev, if you change Format.h its very likely that 
ClangFormatStyleOptions.rst will need updating

This can be done by running clang/docs/tools/dump_format_style.py and that not 
always obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99840

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

[PATCH] D108808: [clang-tidy] bugprone-infinite-loop: Fix false positives with volatile addresses.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

> What happens for something like the following? Maybe it's also worth a test 
> case?

WDYT about the test on line 614?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D108808

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

[PATCH] D108808: [clang-tidy] bugprone-infinite-loop: Fix false positives with volatile addresses.

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D108808#2968742 , @gribozavr2 
wrote:

> In D108808#2968721 , @whisperity 
> wrote:
>
>> What happens for something like the following? Maybe it's also worth a test 
>> case?
>
> WDYT about the test on line 614?

Ah! The fact that the actual value was there misled me.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D108808

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

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: krasimir, curdeius, HazardyKnusperkeks.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

LLVM 13.0.0-rc2 shows change of behaviour in enum and interface BraceWrapping 
(likely before we simply didn't wrap)  but may be related to D99840: 
[clang-format] Correctly attach enum braces with ShortEnums disabled 


Logged as https://bugs.llvm.org/show_bug.cgi?id=51640

This change ensure AfterEnum works for

`internal|public|protected|private enum A {`  in the same way as it works for 
`enum A {` in C++

A similar issue was also observed with `interface` in C#


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108810

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -402,7 +402,9 @@
 }
 
 TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
-  verifyFormat("public enum var {\n"
+  // AfterEnum is true by default.
+  verifyFormat("public enum var\n"
+   "{\n"
"none,\n"
"@string,\n"
"bool,\n"
@@ -1099,5 +1101,194 @@
getGoogleStyle(FormatStyle::LK_Cpp));
 }
 
+TEST_F(FormatTestCSharp, CSharpAfterEnum) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterEnum = false;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("internal enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("public enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("protected enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("private enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterEnum = true;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("internal enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("public enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("protected enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("private enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+}
+
+TEST_F(FormatTestCSharp, CSharpAfterClass) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = false;
+
+  verifyFormat("class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("public class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("protected class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("private class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+
+  verifyFormat("interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("public interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("protected interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("private interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:18472
+  FormatStyle::PCIS_Never);
+
   Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;

HazardyKnusperkeks wrote:
> Should there be a test for the mapping?
Ideally yes, but `CHECK_PARSE` can't check mappings of multiple to one, e.g.:
```
  CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
  "AllowAllConstructorInitializersOnNextLine: false",
  PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108752

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf updated this revision to Diff 369045.
qiucf marked 3 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

Files:
  clang/bindings/python/clang/cindex.py
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/CodeGen/ibm128-cast.c
  clang/test/CodeGen/ibm128-unsupported.c
  clang/test/CodeGenCXX/ibm128-declarations.cpp
  clang/test/Sema/128bitfloat.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -60,6 +60,7 @@
 BTCASE(ULongAccum);
 BTCASE(Float16);
 BTCASE(Float128);
+BTCASE(Ibm128);
 BTCASE(NullPtr);
 BTCASE(Overload);
 BTCASE(Dependent);
@@ -577,6 +578,7 @@
 TKIND(ULongAccum);
 TKIND(Float16);
 TKIND(Float128);
+TKIND(Ibm128);
 TKIND(NullPtr);
 TKIND(Overload);
 TKIND(Dependent);
Index: clang/test/Sema/128bitfloat.cpp
===
--- clang/test/Sema/128bitfloat.cpp
+++ clang/test/Sema/128bitfloat.cpp
@@ -20,7 +20,7 @@
   return x + *y;
 }
 
-// expected-no-diagnostics
+// expected-no-error {{__float128 is not supported on this target}}
 #else
 #if !defined(__STRICT_ANSI__)
 __float128 f;  // expected-error {{__float128 is not supported on this target}}
@@ -44,3 +44,18 @@
 
 #endif
 #endif
+
+#ifdef __ppc__
+__ibm128 i;
+template <> struct __is_floating_point_helper<__ibm128> {};
+int w(int x, __ibm128 *y) {
+  return x + *y;
+}
+// expected-no-error {{__ibm128 is not supported on this target}}
+#else
+__ibm128 i; // expected-error {{__ibm128 is not supported on this target}}
+template <> struct __is_floating_point_helper<__ibm128> {}; // expected-error {{__ibm128 is not supported on this target}}
+int w(int x, __ibm128 *y) { // expected-error {{__ibm128 is not supported on this target}}
+  return x + *y;
+}
+#endif
Index: clang/test/CodeGenCXX/ibm128-declarations.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ibm128-declarations.cpp
@@ -0,0 +1,169 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64-unknown-unknown \
+// RUN:   -std=c++20 %s -o - -debug-info-kind=limited | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64le-unknown-unknown \
+// RUN:   -std=c++20 %s -o - -debug-info-kind=limited | FileCheck %s
+
+#include 
+
+static __ibm128 sgf;
+__ibm128 arrgf[10];
+__ibm128 func1(__ibm128 arg);
+
+class CTest {
+  __ibm128 pf;
+  static const __ibm128 scf;
+  volatile __ibm128 vf;
+
+public:
+  CTest(__ibm128 arg) : pf(arg), vf(arg) {}
+  __ibm128 func2(__ibm128 arg) {
+return pf + arg;
+  }
+  static __ibm128 func3(__ibm128 arg) {
+return arg * CTest::scf;
+  }
+};
+
+constexpr __ibm128 func_add(__ibm128 a, __ibm128 b) {
+  return a + b;
+}
+
+constinit const __ibm128 ci = func_add(1.0, 2.0);
+__ibm128 gf = ci;
+
+__ibm128 func_arith(__ibm128 a, __ibm128 b, __ibm128 c) {
+  __ibm128 v1 = a + b;
+  __ibm128 v2 = a - c;
+  __ibm128 v3 = v1 * c;
+  __ibm128 v4 = v2 / v3;
+  return v4;
+}
+
+__ibm128 func_vaarg(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  __ibm128 r = va_arg(ap, __ibm128);
+  va_end(ap);
+  return r;
+}
+
+template  struct T1 {
+  T mem1;
+};
+template <> struct T1<__ibm128> {
+  __ibm128 mem2;
+};
+
+template <__

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5159
 BT->getKind() == BuiltinType::LongDouble ||
+BT->getKind() == BuiltinType::Ibm128 ||
 (getContext().getTargetInfo().hasFloat128Type() &&

rjmccall wrote:
> qiucf wrote:
> > rjmccall wrote:
> > > I hesitate to ask this, but does `__ibm128` form homogeneous aggregates 
> > > with `double`s?
> > > Homogeneous floating-point aggregates can have up to four IBM EXTENDED 
> > > PRECISION members, four IEEE BINARY 128 QUADRUPLE PRECISION members, four 
> > > _Decimal128 members, or eight members of other floating-point types. 
> > > (Unions are treated as their largest member. For homogeneous unions, 
> > > different union alternatives may have different sizes, provided that all 
> > > union members are homogeneous with respect to each other.) They are 
> > > passed in floating-point registers if parameters of that type would be 
> > > passed in floating-point registers. They are passed in vector registers 
> > > if parameters of that type would be passed in vector registers. They are 
> > > passed as if each member was specified as a separate parameter.
> > 
> > Yes.
> No, I mean, would an `__ibm128` in a struct with a pair of `double`s be 
> treated as if it were 4 `double`s, or is it considered non-homogeneous the 
> same way that a struct with e.g. 4 `float`s and 2 `double`s would be 
> non-homogeneous?
Ah, I think not. `__ibm128` (PowerPC `long double` also produces `ppc_fp128`) 
is distinct type in determining homogeneous aggregates.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 369046.
MyDeveloperDay added a comment.

Remove the unnecessary comment changes


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

https://reviews.llvm.org/D108810

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -402,7 +402,9 @@
 }
 
 TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
-  verifyFormat("public enum var {\n"
+  // AfterEnum is true by default.
+  verifyFormat("public enum var\n"
+   "{\n"
"none,\n"
"@string,\n"
"bool,\n"
@@ -1099,5 +1101,194 @@
getGoogleStyle(FormatStyle::LK_Cpp));
 }
 
+TEST_F(FormatTestCSharp, CSharpAfterEnum) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterEnum = false;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("internal enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("public enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("protected enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("private enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterEnum = true;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("internal enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("public enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("protected enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("private enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+}
+
+TEST_F(FormatTestCSharp, CSharpAfterClass) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = false;
+
+  verifyFormat("class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("public class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("protected class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("private class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+
+  verifyFormat("interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("public interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("protected interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("private interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterClass = true;
+
+  verifyFormat("class MyClass\n"
+   "{\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal class MyClass\n"
+   "{\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("public class MyClass\n"
+   "{\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir accepted this revision.
krasimir added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3803
+  if (Line.startsWith(Keywords.kw_interface) &&
+  Style.BraceWrapping.AfterClass)
+return true;

nit: the `&& Style.BraceWrapping.AfterClass` is unnecessary since we've ensured 
that in the parent `if`. I'd just fold the `kw_interface` case into the 
previous `if`, and then it seems we can further fold the remaining inner `if` 
into the parent `if`, resulting in this whole section (lines 3796--3805) 
turning into a single large `if` statement.


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

https://reviews.llvm.org/D108810

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

[PATCH] D108808: [clang-tidy] bugprone-infinite-loop: Fix false positives with volatile addresses.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D108808#2968721 , @whisperity 
wrote:

> What happens for something like the following? Maybe it's also worth a test 
> case?
>
>   bool wait_analogue_pin(volatile int* address, int threshold) {
> while (*address < threshold) {}
> return true;
>   }

These checkers dodge these cases by noticing that a loop condition relying on 
pointer-type variables, regardless of volatileness, may potentially alias with 
other things in the program and therefore be mutated at any moment without us 
noticing. The problem with concrete addresses wasn't that the check didn't know 
pointers cause problems, but that there weren't any variables in the expression 
to pay attention to. So I taught this sub-system to recognize these 
non-variables.

Banning pointer defererence operations entirely may still be a nicer solution 
that'd also allow us to simplify some code. I'll take a look.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D108808

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

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

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

xbolva00 added a comment.

>> I'll run this warning against ChromeOS and see if I spot any interesting 
>> results.

Thanks!


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

https://reviews.llvm.org/D108003

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

[PATCH] D105269: [X86] AVX512FP16 instructions enabling 6/6

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added a comment.

Thanks for the review.




Comment at: clang/test/CodeGen/X86/avx512fp16-builtins.c:4223
+
+// CFC ADD PH
+

LuoYuanke wrote:
> MADD?
They are marks used when adding tests. We can remove them now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105269

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

[clang] 6ad47e1 - [analyzer] Catch leaking stack addresses via stack variables

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2021-08-27T11:31:16+02:00
New Revision: 6ad47e1c4fbfa8a752cb711acdf242fed3f16a68

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

LOG: [analyzer] Catch leaking stack addresses via stack variables

Not only global variables can hold references to dead stack variables.
Consider this example:

  void write_stack_address_to(char **q) {
char local;
*q = &local;
  }

  void test_stack() {
char *p;
write_stack_address_to(&p);
  }

The address of 'local' is assigned to 'p', which becomes a dangling
pointer after 'write_stack_address_to()' returns.

The StackAddrEscapeChecker was looking for bindings in the store which
referred to variables of the popped stack frame, but it only considered
global variables in this regard. This patch relaxes this, catching
stack variable bindings as well.

---

This patch also works for temporary objects like:

  struct Bar {
const int &ref;
explicit Bar(int y) : ref(y) {
  // Okay.
} // End of the constructor call, `ref` is dangling now. Warning!
  };

  void test() {
Bar{33}; // Temporary object, so the corresponding memregion is
 // *not* a VarRegion.
  }

---

The return value optimization aka. copy-elision might kick in but that
is modeled by passing an imaginary CXXThisRegion which refers to the
parent stack frame which is supposed to be the 'return slot'.
Objects residing in the 'return slot' outlive the scope of the inner
call, thus we should expect no warning about them - except if we
explicitly disable copy-elision.

Reviewed By: NoQ, martong

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
clang/test/Analysis/copy-elision.cpp
clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
clang/test/Analysis/loop-block-counts.c
clang/test/Analysis/stack-addr-ps.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index b5c9356322fcc..d5e86e86424d3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -11,9 +11,9 @@
 //
 
//===--===//
 
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -303,21 +303,53 @@ void StackAddrEscapeChecker::checkEndFunction(const 
ReturnStmt *RS,
   class CallBack : public StoreManager::BindingsHandler {
   private:
 CheckerContext &Ctx;
-const StackFrameContext *CurSFC;
+const StackFrameContext *PoppedFrame;
+
+/// Look for stack variables referring to popped stack variables.
+/// Returns true only if it found some dangling stack variables
+/// referred by an other stack variable from 
diff erent stack frame.
+bool checkForDanglingStackVariable(const MemRegion *Referrer,
+   const MemRegion *Referred) {
+  const auto *ReferrerMemSpace =
+  Referrer->getMemorySpace()->getAs();
+  const auto *ReferredMemSpace =
+  Referred->getMemorySpace()->getAs();
+
+  if (!ReferrerMemSpace || !ReferredMemSpace)
+return false;
+
+  const auto *ReferrerFrame = ReferrerMemSpace->getStackFrame();
+  const auto *ReferredFrame = ReferredMemSpace->getStackFrame();
+
+  if (ReferrerMemSpace && ReferredMemSpace) {
+if (ReferredFrame == PoppedFrame &&
+ReferrerFrame->isParentOf(PoppedFrame)) {
+  V.emplace_back(Referrer, Referred);
+  return true;
+}
+  }
+  return false;
+}
 
   public:
 SmallVector, 10> V;
 
-CallBack(CheckerContext &CC) : Ctx(CC), CurSFC(CC.getStackFrame()) {}
+CallBack(CheckerContext &CC) : Ctx(CC), PoppedFrame(CC.getStackFrame()) {}
 
 bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Region,
SVal Val) override {
+  const MemRegion *VR = Val.getAsRegion();
+  if (!VR)
+return true;
+
+  if (checkForDanglingStackVariable(Region, VR))
+return true;
 
+  // Check the globals for the same.
   if (!isa(Region->getMemorySpace()))
 return true;
-  const MemRegion *VR = Val.getAsRegion();
-  if (VR && isa(VR->getMemorySpace()) &&
-  !isArcManagedBlock(VR, Ctx) && !isNotInCurrentFrame(VR, Ctx))
+ 

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6ad47e1c4fbf: [analyzer] Catch leaking stack addresses via 
stack variables (authored by steakhal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107078

Files:
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/test/Analysis/copy-elision.cpp
  clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
  clang/test/Analysis/loop-block-counts.c
  clang/test/Analysis/stack-addr-ps.cpp

Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -137,3 +137,28 @@
   }
 }
 
+void write_stack_address_to(char **q) {
+  char local;
+  *q = &local;
+  // expected-warning@-1 {{Address of stack memory associated with local \
+variable 'local' is still referred to by the stack variable 'p' upon \
+returning to the caller}}
+}
+
+void test_stack() {
+  char *p;
+  write_stack_address_to(&p);
+}
+
+struct C {
+  ~C() {} // non-trivial class
+};
+
+C make1() {
+  C c;
+  return c; // no-warning
+}
+
+void test_copy_elision() {
+  C c1 = make1();
+}
Index: clang/test/Analysis/loop-block-counts.c
===
--- clang/test/Analysis/loop-block-counts.c
+++ clang/test/Analysis/loop-block-counts.c
@@ -5,6 +5,9 @@
 void callee(void **p) {
   int x;
   *p = &x;
+  // expected-warning@-1 {{Address of stack memory associated with local \
+variable 'x' is still referred to by the stack variable 'arr' upon \
+returning to the caller}}
 }
 
 void loop() {
Index: clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
===
--- clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -71,6 +71,9 @@
   void *allocaPtr;
   int dontGetFilteredByNonPedanticMode = 0;
 
+  // expected-warning-re@+3 {{Address of stack memory allocated by call to \
+alloca() on line {{[0-9]+}} is still referred to by a temporary object on the \
+stack upon returning to the caller.  This will be a dangling reference}}
   UntypedAllocaTest() : allocaPtr(__builtin_alloca(sizeof(int))) {
 // All good!
   }
@@ -86,6 +89,9 @@
 
   TypedAllocaTest1() // expected-warning{{1 uninitialized field}}
   : allocaPtr(static_cast(__builtin_alloca(sizeof(int {}
+  // expected-warning-re@-2 {{Address of stack memory allocated by call to \
+alloca() on line {{[0-9]+}} is still referred to by a temporary object on the \
+stack upon returning to the caller.  This will be a dangling reference}}
 };
 
 void fTypedAllocaTest1() {
@@ -96,6 +102,9 @@
   int *allocaPtr;
   int dontGetFilteredByNonPedanticMode = 0;
 
+  // expected-warning-re@+5 {{Address of stack memory allocated by call to \
+alloca() on line {{[0-9]+}} is still referred to by a temporary object on the \
+stack upon returning to the caller.  This will be a dangling reference}}
   TypedAllocaTest2()
   : allocaPtr(static_cast(__builtin_alloca(sizeof(int {
 *allocaPtr = 5;
@@ -341,6 +350,9 @@
   void *&&vptrrref; // expected-note {{here}}
 
 public:
+  // expected-warning@+3 {{Address of stack memory associated with local \
+variable 'vptr' is still referred to by a temporary object on the stack \
+upon returning to the caller.  This will be a dangling reference}}
   VoidPointerRRefTest1(void *vptr, char) : vptrrref(static_cast(vptr)) { // expected-warning {{binding reference member 'vptrrref' to stack allocated parameter 'vptr'}}
 // All good!
   }
@@ -355,6 +367,9 @@
   void **&&vpptrrref; // expected-note {{here}}
 
 public:
+  // expected-warning@+3 {{Address of stack memory associated with local \
+variable 'vptr' is still referred to by a temporary object on the stack \
+upon returning to the caller.  This will be a dangling reference}}
   VoidPointerRRefTest2(void **vptr, char) : vpptrrref(static_cast(vptr)) { // expected-warning {{binding reference member 'vpptrrref' to stack allocated parameter 'vptr'}}
 // All good!
   }
@@ -369,6 +384,9 @@
   void *&vptrrref; // expected-note {{here}}
 
 public:
+  // expected-warning@+3 {{Address of stack memory associated with local \
+variable 'vptr' is still referred to by a temporary object on the stack \
+upon returning to the caller.  This will be a dangling reference}}
   VoidPointerLRefTest(void *vptr, char) : vptrrref(static_cast(vptr)) { // expected-warning {{binding reference member 'vptrrref' to stack allocated parameter 'vptr'}}
 // All good!
   }
Index: clang/test/Analysis/copy-elision.cpp
===
--- clang/test/Analysis/copy-elision.cpp
+++ clang/test/Analysis/copy-elision.cpp
@@ -1,7 +1,13 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 369052.
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.

Address Nit to reduce if nesting

  [--] Global test environment tear-down
  [==] 814 tests from 25 test suites ran. (41295 ms total)
  [  PASSED  ] 814 tests.


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

https://reviews.llvm.org/D108810

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -402,7 +402,9 @@
 }
 
 TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
-  verifyFormat("public enum var {\n"
+  // AfterEnum is true by default.
+  verifyFormat("public enum var\n"
+   "{\n"
"none,\n"
"@string,\n"
"bool,\n"
@@ -1099,5 +1101,194 @@
getGoogleStyle(FormatStyle::LK_Cpp));
 }
 
+TEST_F(FormatTestCSharp, CSharpAfterEnum) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterEnum = false;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("internal enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("public enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("protected enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("private enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterEnum = true;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("internal enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("public enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("protected enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("private enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+}
+
+TEST_F(FormatTestCSharp, CSharpAfterClass) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = false;
+
+  verifyFormat("class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("public class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("protected class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("private class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+
+  verifyFormat("interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("public interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("protected interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("private interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterClass = true;
+
+  verifyFormat("class MyClass\n"
+   "{\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal class MyClass\n"
+   "{\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3803
+  if (Line.startsWith(Keywords.kw_interface) &&
+  Style.BraceWrapping.AfterClass)
+return true;

krasimir wrote:
> nit: the `&& Style.BraceWrapping.AfterClass` is unnecessary since we've 
> ensured that in the parent `if`. I'd just fold the `kw_interface` case into 
> the previous `if`, and then it seems we can further fold the remaining inner 
> `if` into the parent `if`, resulting in this whole section (lines 3796--3805) 
> turning into a single large `if` statement.
nice catch, will do


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

https://reviews.llvm.org/D108810

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

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/Format.cpp:666-677
+if (Style.PackConstructorInitializers == FormatStyle::PCIS_BinPack) {
+  bool OnCurrentLine = false;
+  IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
+ OnCurrentLine);
+  if (OnCurrentLine) {
+bool OnNextLine = false;
+IO.mapOptional("AllowAllConstructorInitializersOnNextLine", 
OnNextLine);

Also need to handle the default value `PCIS_NextLine` for Google and Chromium 
styles:
```
StringRef BasedOn;
IO.mapOptional("BasedOnStyle", BasedOn);
const bool IsGoogleOrChromium = BasedOn.equals_insensitive("google") ||
BasedOn.equals_insensitive("chromium");
bool OnCurrentLine = IsGoogleOrChromium;
bool OnNextLine = IsGoogleOrChromium;
IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
   OnCurrentLine);
IO.mapOptional("AllowAllConstructorInitializersOnNextLine", OnNextLine);
if (IsGoogleOrChromium &&
Style.PackConstructorInitializers == FormatStyle::PCIS_NextLine) {
  if (!OnCurrentLine)
Style.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
  else if (!OnNextLine)
Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
} else if (Style.PackConstructorInitializers == FormatStyle::PCIS_BinPack &&
   OnCurrentLine) {
  Style.PackConstructorInitializers = OnNextLine
  ? FormatStyle::PCIS_NextLine
  : FormatStyle::PCIS_CurrentLine;
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108752

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

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/Format.cpp:1189
   GoogleStyle.DerivePointerAlignment = true;
+  GoogleStyle.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
   GoogleStyle.IncludeStyle.IncludeCategories = {{"^", 2, 0, false},

Need to move this line down to keep the options sorted.



Comment at: clang/unittests/Format/FormatTest.cpp:18463-18471
+  Style.PackConstructorInitializers = FormatStyle::PCIS_Never;
+  CHECK_PARSE("PackConstructorInitializers: BinPack",
+  PackConstructorInitializers, FormatStyle::PCIS_BinPack);
+  CHECK_PARSE("PackConstructorInitializers: CurrentLine",
+  PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
+  CHECK_PARSE("PackConstructorInitializers: NextLine",
+  PackConstructorInitializers, FormatStyle::PCIS_NextLine);

Will use the default `PCIS_BinPack` as the initial value and move the last 
`CHECK_PARSE` to the top.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108752

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

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 369061.
owenpan added a comment.

Added backward compatibility for the default value `PCIS_NextLine` in Google 
and Chromium styles and aforementioned cleanups.


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

https://reviews.llvm.org/D108752

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5960,7 +5960,26 @@
"  at() {}");
 
   FormatStyle OnePerLine = getLLVMStyle();
-  OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  OnePerLine.PackConstructorInitializers = FormatStyle::PCIS_Never;
+  verifyFormat("MyClass::MyClass()\n"
+   ": a(a),\n"
+   "  b(b),\n"
+   "  c(c) {}",
+   OnePerLine);
+  verifyFormat("MyClass::MyClass()\n"
+   ": a(a), // comment\n"
+   "  b(b),\n"
+   "  c(c) {}",
+   OnePerLine);
+  verifyFormat("MyClass::MyClass(int a)\n"
+   ": b(a),  // comment\n"
+   "  c(a + 1) { // lined up\n"
+   "}",
+   OnePerLine);
+  verifyFormat("Constructor()\n"
+   ": a(b, b, b) {}",
+   OnePerLine);
+  OnePerLine.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
   OnePerLine.AllowAllParametersOfDeclarationOnNextLine = false;
   verifyFormat("SomeClass::Constructor()\n"
": a(aa),\n"
@@ -6013,8 +6032,6 @@
   FormatStyle Style = getLLVMStyle();
   Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
   Style.ColumnLimit = 60;
-  Style.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
-  Style.AllowAllConstructorInitializersOnNextLine = true;
   Style.BinPackParameters = false;
 
   for (int i = 0; i < 4; ++i) {
@@ -6022,14 +6039,14 @@
 Style.AllowAllParametersOfDeclarationOnNextLine = i & 1;
 Style.AllowAllArgumentsOnNextLine = i & 2;
 
-Style.AllowAllConstructorInitializersOnNextLine = true;
+Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
 Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
 verifyFormat("Constructor()\n"
  ": (a), b(b) {}",
  Style);
 verifyFormat("Constructor() : a(a), b(b) {}", Style);
 
-Style.AllowAllConstructorInitializersOnNextLine = false;
+Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
 verifyFormat("Constructor()\n"
  ": (a)\n"
  ", b(b) {}",
@@ -6037,24 +6054,24 @@
 verifyFormat("Constructor() : a(a), b(b) {}", Style);
 
 Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
-Style.AllowAllConstructorInitializersOnNextLine = true;
+Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
 verifyFormat("Constructor()\n"
  ": (a), b(b) {}",
  Style);
 
-Style.AllowAllConstructorInitializersOnNextLine = false;
+Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
 verifyFormat("Constructor()\n"
  ": (a),\n"
  "  b(b) {}",
  Style);
 
 Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon;
-Style.AllowAllConstructorInitializersOnNextLine = true;
+Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
 verifyFormat("Constructor() :\n"
  "aa(a), b(b) {}",
  Style);
 
-Style.AllowAllConstructorInitializersOnNextLine = false;
+Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
 verifyFormat("Constructor() :\n"
  "aa(a),\n"
  "b(b) {}",
@@ -6066,14 +6083,13 @@
   // BreakConstructorInitializers modes
   Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
   Style.AllowAllParametersOfDeclarationOnNextLine = true;
-  Style.AllowAllConstructorInitializersOnNextLine = false;
   verifyFormat("SomeClassWithALongName::Constructor(\n"
"int , int b)\n"
": (a)\n"
", b(b) {}",
Style);
 
-  Style.AllowAllConstructorInitializersOnNextLine = true

[clang] 8d3f112 - [M68k] Update pointer data layout

[Ricky Taylor via cfe-commits]

Author: Ricky Taylor
Date: 2021-08-27T11:47:27+01:00
New Revision: 8d3f112f0cdbed2311aead86bcd72e763ad55255

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

LOG: [M68k] Update pointer data layout

Fixes PR51626.

The M68k requires that all instruction, word and long word reads are
aligned to word boundaries. From the 68020 onwards, there is a
performance benefit from aligning long words to long word boundaries.

The M68k uses the same data layout for pointers and integers.

In line with this, this commit updates the pointer data layout to
match the layout already set for 32-bit integers: 32:16:32.

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

Added: 


Modified: 
clang/lib/Basic/Targets/M68k.cpp
llvm/lib/Target/M68k/M68kTargetMachine.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/M68k.cpp 
b/clang/lib/Basic/Targets/M68k.cpp
index 31cb36d37636c..c0cd8fa90ed6b 100644
--- a/clang/lib/Basic/Targets/M68k.cpp
+++ b/clang/lib/Basic/Targets/M68k.cpp
@@ -37,8 +37,8 @@ M68kTargetInfo::M68kTargetInfo(const llvm::Triple &Triple,
   // FIXME how to wire it with the used object format?
   Layout += "-m:e";
 
-  // M68k pointers are always 32 bit wide even for 16 bit cpus
-  Layout += "-p:32:32";
+  // M68k pointers are always 32 bit wide even for 16-bit CPUs
+  Layout += "-p:32:16:32";
 
   // M68k integer data types
   Layout += "-i8:8:8-i16:16:16-i32:16:32";

diff  --git a/llvm/lib/Target/M68k/M68kTargetMachine.cpp 
b/llvm/lib/Target/M68k/M68kTargetMachine.cpp
index 5b8fd3d41b147..cb7d8f8b25e39 100644
--- a/llvm/lib/Target/M68k/M68kTargetMachine.cpp
+++ b/llvm/lib/Target/M68k/M68kTargetMachine.cpp
@@ -49,10 +49,14 @@ std::string computeDataLayout(const Triple &TT, StringRef 
CPU,
   // FIXME how to wire it with the used object format?
   Ret += "-m:e";
 
-  // M68k pointers are always 32 bit wide even for 16 bit cpus
-  Ret += "-p:32:32";
-
-  // M68k requires i8 to align on 2 byte boundry
+  // M68k pointers are always 32 bit wide even for 16-bit CPUs.
+  // The ABI only specifies 16-bit alignment.
+  // On at least the 68020+ with a 32-bit bus, there is a performance benefit
+  // to having 32-bit alignment.
+  Ret += "-p:32:16:32";
+
+  // Bytes do not require special alignment, words are word aligned and
+  // long words are word aligned at minimum.
   Ret += "-i8:8:8-i16:16:16-i32:16:32";
 
   // FIXME no floats at the moment



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

[PATCH] D108792: [M68k] Update pointer data layout

[Ricky Taylor 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 rG8d3f112f0cdb: [M68k] Update pointer data layout (authored by 
ricky26).

Changed prior to commit:
  https://reviews.llvm.org/D108792?vs=368995&id=369064#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108792

Files:
  clang/lib/Basic/Targets/M68k.cpp
  llvm/lib/Target/M68k/M68kTargetMachine.cpp


Index: llvm/lib/Target/M68k/M68kTargetMachine.cpp
===
--- llvm/lib/Target/M68k/M68kTargetMachine.cpp
+++ llvm/lib/Target/M68k/M68kTargetMachine.cpp
@@ -49,10 +49,14 @@
   // FIXME how to wire it with the used object format?
   Ret += "-m:e";
 
-  // M68k pointers are always 32 bit wide even for 16 bit cpus
-  Ret += "-p:32:32";
-
-  // M68k requires i8 to align on 2 byte boundry
+  // M68k pointers are always 32 bit wide even for 16-bit CPUs.
+  // The ABI only specifies 16-bit alignment.
+  // On at least the 68020+ with a 32-bit bus, there is a performance benefit
+  // to having 32-bit alignment.
+  Ret += "-p:32:16:32";
+
+  // Bytes do not require special alignment, words are word aligned and
+  // long words are word aligned at minimum.
   Ret += "-i8:8:8-i16:16:16-i32:16:32";
 
   // FIXME no floats at the moment
Index: clang/lib/Basic/Targets/M68k.cpp
===
--- clang/lib/Basic/Targets/M68k.cpp
+++ clang/lib/Basic/Targets/M68k.cpp
@@ -37,8 +37,8 @@
   // FIXME how to wire it with the used object format?
   Layout += "-m:e";
 
-  // M68k pointers are always 32 bit wide even for 16 bit cpus
-  Layout += "-p:32:32";
+  // M68k pointers are always 32 bit wide even for 16-bit CPUs
+  Layout += "-p:32:16:32";
 
   // M68k integer data types
   Layout += "-i8:8:8-i16:16:16-i32:16:32";


Index: llvm/lib/Target/M68k/M68kTargetMachine.cpp
===
--- llvm/lib/Target/M68k/M68kTargetMachine.cpp
+++ llvm/lib/Target/M68k/M68kTargetMachine.cpp
@@ -49,10 +49,14 @@
   // FIXME how to wire it with the used object format?
   Ret += "-m:e";
 
-  // M68k pointers are always 32 bit wide even for 16 bit cpus
-  Ret += "-p:32:32";
-
-  // M68k requires i8 to align on 2 byte boundry
+  // M68k pointers are always 32 bit wide even for 16-bit CPUs.
+  // The ABI only specifies 16-bit alignment.
+  // On at least the 68020+ with a 32-bit bus, there is a performance benefit
+  // to having 32-bit alignment.
+  Ret += "-p:32:16:32";
+
+  // Bytes do not require special alignment, words are word aligned and
+  // long words are word aligned at minimum.
   Ret += "-i8:8:8-i16:16:16-i32:16:32";
 
   // FIXME no floats at the moment
Index: clang/lib/Basic/Targets/M68k.cpp
===
--- clang/lib/Basic/Targets/M68k.cpp
+++ clang/lib/Basic/Targets/M68k.cpp
@@ -37,8 +37,8 @@
   // FIXME how to wire it with the used object format?
   Layout += "-m:e";
 
-  // M68k pointers are always 32 bit wide even for 16 bit cpus
-  Layout += "-p:32:32";
+  // M68k pointers are always 32 bit wide even for 16-bit CPUs
+  Layout += "-p:32:16:32";
 
   // M68k integer data types
   Layout += "-i8:8:8-i16:16:16-i32:16:32";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D108752#2967040 , @MyDeveloperDay 
wrote:

> This looks good, I like to move towards one style, it was getting too 
> confusing.

I agree!

> Did you test this on a large code base at all?

No. I don't think it's necessary (as this patch doesn't impact breaking 
constructor initializers except perhaps for the new value `Never`, which is 
already covered by the new test cases) but will try.

> I'll pull the patch to my local source and see if it makes any changes, but 
> to be honest we only use the `BreakConstructorInitializersBeforeComma` option

Thanks!


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

https://reviews.llvm.org/D108752

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

[PATCH] D108708: [openmp][amdgpu] Initial gfx10 offloading implementation

[Ron Lieberman via Phabricator via cfe-commits]

ronlieb added inline comments.



Comment at: openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt:111
+set(mcpus gfx700 gfx701 gfx801 gfx803 gfx900 gfx902 gfx906 gfx908 gfx1010 
gfx1031)
 if (DEFINED LIBOMPTARGET_AMDGCN_GFXLIST)
   set(mcpus ${LIBOMPTARGET_AMDGCN_GFXLIST})

JonChesterfield wrote:
> I've got a gfx1010 locally and @dpalermo has a gfx1031
gfx1030 is also valid, please add



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108708

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

[PATCH] D108708: [openmp][amdgpu] Initial gfx10 offloading implementation

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield updated this revision to Diff 369067.
JonChesterfield added a comment.

- uint32 to unsigned


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108708

Files:
  clang/lib/Basic/Targets/AMDGPU.h
  llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h
  openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt
  openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h
  openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

Index: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
===
--- openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
+++ openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
@@ -436,8 +436,9 @@
   int MaxTeamsDefault;
 };
 
+template 
 static constexpr const llvm::omp::GV &getGridValue() {
-  return llvm::omp::AMDGPUGridValues;
+  return llvm::omp::getAMDGPUGridValues();
 }
 
 /// Class containing all the device information
@@ -508,10 +509,24 @@
   static const unsigned HardTeamLimit =
   (1 << 16) - 1; // 64K needed to fit in uint16
   static const int DefaultNumTeams = 128;
-  static const int Max_Teams = getGridValue().GV_Max_Teams;
-  static const int Warp_Size = getGridValue().GV_Warp_Size;
-  static const int Max_WG_Size = getGridValue().GV_Max_WG_Size;
-  static const int Default_WG_Size = getGridValue().GV_Default_WG_Size;
+
+  // These need to be per-device since different devices can have different
+  // wave sizes, but are currently the same number for each so that refactor
+  // can be postponed.
+  static_assert(getGridValue<32>().GV_Max_Teams ==
+getGridValue<64>().GV_Max_Teams,
+"");
+  static const int Max_Teams = getGridValue<64>().GV_Max_Teams;
+
+  static_assert(getGridValue<32>().GV_Max_WG_Size ==
+getGridValue<64>().GV_Max_WG_Size,
+"");
+  static const int Max_WG_Size = getGridValue<64>().GV_Max_WG_Size;
+
+  static_assert(getGridValue<32>().GV_Default_WG_Size ==
+getGridValue<64>().GV_Default_WG_Size,
+"");
+  static const int Default_WG_Size = getGridValue<64>().GV_Default_WG_Size;
 
   using MemcpyFunc = hsa_status_t (*)(hsa_signal_t, void *, const void *,
   size_t size, hsa_agent_t,
@@ -1060,8 +1075,9 @@
 DP("Queried wavefront size: %d\n", wavefront_size);
 DeviceInfo.WarpSize[device_id] = wavefront_size;
   } else {
-DP("Default wavefront size: %d\n", getGridValue().GV_Warp_Size);
-DeviceInfo.WarpSize[device_id] = getGridValue().GV_Warp_Size;
+// TODO: Burn the wavefront size into the code object
+DP("Warning: Unknown wavefront size, assuming 64\n");
+DeviceInfo.WarpSize[device_id] = 64;
   }
 
   // Adjust teams to the env variables
@@ -1886,9 +1902,10 @@
   int WorkgroupSize;
   int GridSize;
 };
-launchVals getLaunchVals(EnvironmentVariables Env, int ConstWGSize,
- int ExecutionMode, int num_teams, int thread_limit,
- uint64_t loop_tripcount, int DeviceNumTeams) {
+launchVals getLaunchVals(int WarpSize, EnvironmentVariables Env,
+ int ConstWGSize, int ExecutionMode, int num_teams,
+ int thread_limit, uint64_t loop_tripcount,
+ int DeviceNumTeams) {
 
   int threadsPerGroup = RTLDeviceInfoTy::Default_WG_Size;
   int num_groups = 0;
@@ -1901,7 +1918,7 @@
   if (print_kernel_trace & STARTUP_DETAILS) {
 DP("RTLDeviceInfoTy::Max_Teams: %d\n", RTLDeviceInfoTy::Max_Teams);
 DP("Max_Teams: %d\n", Max_Teams);
-DP("RTLDeviceInfoTy::Warp_Size: %d\n", RTLDeviceInfoTy::Warp_Size);
+DP("RTLDeviceInfoTy::Warp_Size: %d\n", WarpSize);
 DP("RTLDeviceInfoTy::Max_WG_Size: %d\n", RTLDeviceInfoTy::Max_WG_Size);
 DP("RTLDeviceInfoTy::Default_WG_Size: %d\n",
RTLDeviceInfoTy::Default_WG_Size);
@@ -1914,8 +1931,8 @@
 threadsPerGroup = thread_limit;
 DP("Setting threads per block to requested %d\n", thread_limit);
 if (ExecutionMode == GENERIC) { // Add master warp for GENERIC
-  threadsPerGroup += RTLDeviceInfoTy::Warp_Size;
-  DP("Adding master wavefront: +%d threads\n", RTLDeviceInfoTy::Warp_Size);
+  threadsPerGroup += WarpSize;
+  DP("Adding master wavefront: +%d threads\n", WarpSize);
 }
 if (threadsPerGroup > RTLDeviceInfoTy::Max_WG_Size) { // limit to max
   threadsPerGroup = RTLDeviceInfoTy::Max_WG_Size;
@@ -1951,7 +1968,7 @@
   // So we only handle constant thread_limits.
   if (threadsPerGroup >
   RTLDeviceInfoTy::Default_WG_Size) //  256 < threadsPerGroup <= 1024
-// Should we round threadsPerGroup up to nearest RTLDeviceInfoTy::Warp_Size
+// Should we round threadsPerGroup up to nearest WarpSize
 // here?
 num_groups = (Max_Teams * RTLDeviceInfoTy::Max_WG_Size) / threadsPerGroup;
 
@@ -2100,12 +2117,13 @@
   /*
* Set limit based on ThreadsPerGroup and Group

[PATCH] D108708: [openmp][amdgpu] Initial gfx10 offloading implementation

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield updated this revision to Diff 369069.
JonChesterfield added a comment.

- add 1030


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108708

Files:
  clang/lib/Basic/Targets/AMDGPU.h
  llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h
  openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt
  openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h
  openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

Index: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
===
--- openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
+++ openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
@@ -436,8 +436,9 @@
   int MaxTeamsDefault;
 };
 
+template 
 static constexpr const llvm::omp::GV &getGridValue() {
-  return llvm::omp::AMDGPUGridValues;
+  return llvm::omp::getAMDGPUGridValues();
 }
 
 /// Class containing all the device information
@@ -508,10 +509,24 @@
   static const unsigned HardTeamLimit =
   (1 << 16) - 1; // 64K needed to fit in uint16
   static const int DefaultNumTeams = 128;
-  static const int Max_Teams = getGridValue().GV_Max_Teams;
-  static const int Warp_Size = getGridValue().GV_Warp_Size;
-  static const int Max_WG_Size = getGridValue().GV_Max_WG_Size;
-  static const int Default_WG_Size = getGridValue().GV_Default_WG_Size;
+
+  // These need to be per-device since different devices can have different
+  // wave sizes, but are currently the same number for each so that refactor
+  // can be postponed.
+  static_assert(getGridValue<32>().GV_Max_Teams ==
+getGridValue<64>().GV_Max_Teams,
+"");
+  static const int Max_Teams = getGridValue<64>().GV_Max_Teams;
+
+  static_assert(getGridValue<32>().GV_Max_WG_Size ==
+getGridValue<64>().GV_Max_WG_Size,
+"");
+  static const int Max_WG_Size = getGridValue<64>().GV_Max_WG_Size;
+
+  static_assert(getGridValue<32>().GV_Default_WG_Size ==
+getGridValue<64>().GV_Default_WG_Size,
+"");
+  static const int Default_WG_Size = getGridValue<64>().GV_Default_WG_Size;
 
   using MemcpyFunc = hsa_status_t (*)(hsa_signal_t, void *, const void *,
   size_t size, hsa_agent_t,
@@ -1060,8 +1075,9 @@
 DP("Queried wavefront size: %d\n", wavefront_size);
 DeviceInfo.WarpSize[device_id] = wavefront_size;
   } else {
-DP("Default wavefront size: %d\n", getGridValue().GV_Warp_Size);
-DeviceInfo.WarpSize[device_id] = getGridValue().GV_Warp_Size;
+// TODO: Burn the wavefront size into the code object
+DP("Warning: Unknown wavefront size, assuming 64\n");
+DeviceInfo.WarpSize[device_id] = 64;
   }
 
   // Adjust teams to the env variables
@@ -1886,9 +1902,10 @@
   int WorkgroupSize;
   int GridSize;
 };
-launchVals getLaunchVals(EnvironmentVariables Env, int ConstWGSize,
- int ExecutionMode, int num_teams, int thread_limit,
- uint64_t loop_tripcount, int DeviceNumTeams) {
+launchVals getLaunchVals(int WarpSize, EnvironmentVariables Env,
+ int ConstWGSize, int ExecutionMode, int num_teams,
+ int thread_limit, uint64_t loop_tripcount,
+ int DeviceNumTeams) {
 
   int threadsPerGroup = RTLDeviceInfoTy::Default_WG_Size;
   int num_groups = 0;
@@ -1901,7 +1918,7 @@
   if (print_kernel_trace & STARTUP_DETAILS) {
 DP("RTLDeviceInfoTy::Max_Teams: %d\n", RTLDeviceInfoTy::Max_Teams);
 DP("Max_Teams: %d\n", Max_Teams);
-DP("RTLDeviceInfoTy::Warp_Size: %d\n", RTLDeviceInfoTy::Warp_Size);
+DP("RTLDeviceInfoTy::Warp_Size: %d\n", WarpSize);
 DP("RTLDeviceInfoTy::Max_WG_Size: %d\n", RTLDeviceInfoTy::Max_WG_Size);
 DP("RTLDeviceInfoTy::Default_WG_Size: %d\n",
RTLDeviceInfoTy::Default_WG_Size);
@@ -1914,8 +1931,8 @@
 threadsPerGroup = thread_limit;
 DP("Setting threads per block to requested %d\n", thread_limit);
 if (ExecutionMode == GENERIC) { // Add master warp for GENERIC
-  threadsPerGroup += RTLDeviceInfoTy::Warp_Size;
-  DP("Adding master wavefront: +%d threads\n", RTLDeviceInfoTy::Warp_Size);
+  threadsPerGroup += WarpSize;
+  DP("Adding master wavefront: +%d threads\n", WarpSize);
 }
 if (threadsPerGroup > RTLDeviceInfoTy::Max_WG_Size) { // limit to max
   threadsPerGroup = RTLDeviceInfoTy::Max_WG_Size;
@@ -1951,7 +1968,7 @@
   // So we only handle constant thread_limits.
   if (threadsPerGroup >
   RTLDeviceInfoTy::Default_WG_Size) //  256 < threadsPerGroup <= 1024
-// Should we round threadsPerGroup up to nearest RTLDeviceInfoTy::Warp_Size
+// Should we round threadsPerGroup up to nearest WarpSize
 // here?
 num_groups = (Max_Teams * RTLDeviceInfoTy::Max_WG_Size) / threadsPerGroup;
 
@@ -2100,12 +2117,13 @@
   /*
* Set limit based on ThreadsPerGroup and GroupsPerDevice

[PATCH] D108708: [openmp][amdgpu] Initial gfx10 offloading implementation

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield marked an inline comment as done.
JonChesterfield added a subscriber: dhruvachak.
JonChesterfield added inline comments.



Comment at: openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt:111
+set(mcpus gfx700 gfx701 gfx801 gfx803 gfx900 gfx902 gfx906 gfx908 gfx1010 
gfx1031)
 if (DEFINED LIBOMPTARGET_AMDGCN_GFXLIST)
   set(mcpus ${LIBOMPTARGET_AMDGCN_GFXLIST})

ronlieb wrote:
> JonChesterfield wrote:
> > I've got a gfx1010 locally and @dpalermo has a gfx1031
> gfx1030 is also valid, please add
> 
we think @dhruvachak has a 1030, added


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108708

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

[PATCH] D108708: [openmp][amdgpu] Initial gfx10 offloading implementation

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:1921
 DP("Max_Teams: %d\n", Max_Teams);
-DP("RTLDeviceInfoTy::Warp_Size: %d\n", RTLDeviceInfoTy::Warp_Size);
+DP("RTLDeviceInfoTy::Warp_Size: %d\n", WarpSize);
 DP("RTLDeviceInfoTy::Max_WG_Size: %d\n", RTLDeviceInfoTy::Max_WG_Size);

This part is strictly a bugfix - previously we printed the default wave size, 
now we print the one that is in use


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108708

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

[PATCH] D108708: [openmp][amdgpu] Initial gfx10 offloading implementation

[Ron Lieberman via Phabricator via cfe-commits]

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

since amdgpu buildbots are red, at Jon's request, i applied patch on latest 
trunk build.
applies clean, builds fine, no additional regressions in bheck-openmp

compiler now accepts -march=gfx1030 and gfx1031


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108708

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

[clang] 78f92c3 - [openmp][amdgpu] Initial gfx10 offloading implementation

[Jon Chesterfield via cfe-commits]

Author: Jon Chesterfield
Date: 2021-08-27T12:34:03+01:00
New Revision: 78f92c38101fd1f6788500b3362d3c9c28213bc0

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

LOG: [openmp][amdgpu] Initial gfx10 offloading implementation

Lets wavefront size be 32 for amdgpu openmp, as well as 64.

Fixes up as little as possible to pass that through the libraries. This change
is end to end, as opposed to updating clang/devicertl/plugin separately. It can
be broken up for review/commit if preferred. Posting as-is so that others with
a gfx10 can try it out. It works roughly as well as gfx9 for me, but there are
probably bugs remaining as well as the todo: for letting grid values vary more.

Reviewed By: ronlieb

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

Added: 


Modified: 
clang/lib/Basic/Targets/AMDGPU.h
llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h
openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt
openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h
openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/AMDGPU.h 
b/clang/lib/Basic/Targets/AMDGPU.h
index e791a83f38ae7..700b76452eea2 100644
--- a/clang/lib/Basic/Targets/AMDGPU.h
+++ b/clang/lib/Basic/Targets/AMDGPU.h
@@ -371,7 +371,14 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : 
public TargetInfo {
   }
 
   const llvm::omp::GV &getGridValue() const override {
-return llvm::omp::AMDGPUGridValues;
+switch (WavefrontSize) {
+case 32:
+  return llvm::omp::getAMDGPUGridValues<32>();
+case 64:
+  return llvm::omp::getAMDGPUGridValues<64>();
+default:
+  llvm_unreachable("getGridValue not implemented for this wavesize");
+}
   }
 
   /// \returns Target specific vtbl ptr address space.

diff  --git a/llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h 
b/llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h
index f5674e443b67e..89f5de229b3b1 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h
@@ -81,7 +81,7 @@ struct GV {
 };
 
 /// For AMDGPU GPUs
-static constexpr GV AMDGPUGridValues = {
+static constexpr GV AMDGPUGridValues64 = {
 256,  // GV_Slot_Size
 64,   // GV_Warp_Size
 128,  // GV_Max_Teams
@@ -90,6 +90,20 @@ static constexpr GV AMDGPUGridValues = {
 256,  // GV_Default_WG_Size
 };
 
+static constexpr GV AMDGPUGridValues32 = {
+256,  // GV_Slot_Size
+32,   // GV_Warp_Size
+128,  // GV_Max_Teams
+896,  // GV_SimpleBufferSize
+1024, // GV_Max_WG_Size,
+256,  // GV_Default_WG_Size
+};
+
+template  constexpr const GV &getAMDGPUGridValues() {
+  static_assert(wavesize == 32 || wavesize == 64, "");
+  return wavesize == 32 ? AMDGPUGridValues32 : AMDGPUGridValues64;
+}
+
 /// For Nvidia GPUs
 static constexpr GV NVPTXGridValues = {
 256,  // GV_Slot_Size

diff  --git a/openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt 
b/openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt
index 7d10b1edabe04..a0aa471594f70 100644
--- a/openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt
+++ b/openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt
@@ -107,7 +107,7 @@ else()
 endif()
 
 # create libraries
-set(mcpus gfx700 gfx701 gfx801 gfx803 gfx900 gfx902 gfx906 gfx908)
+set(mcpus gfx700 gfx701 gfx801 gfx803 gfx900 gfx902 gfx906 gfx908 gfx1010 
gfx1030 gfx1031)
 if (DEFINED LIBOMPTARGET_AMDGCN_GFXLIST)
   set(mcpus ${LIBOMPTARGET_AMDGCN_GFXLIST})
 endif()

diff  --git a/openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h 
b/openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h
index 5396590d46e00..f4755996c86aa 100644
--- a/openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h
+++ b/openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h
@@ -38,7 +38,7 @@ typedef uint64_t __kmpc_impl_lanemask_t;
 #include "llvm/Frontend/OpenMP/OMPGridValues.h"
 
 INLINE constexpr const llvm::omp::GV &getGridValue() {
-  return llvm::omp::AMDGPUGridValues;
+  return llvm::omp::getAMDGPUGridValues<__AMDGCN_WAVEFRONT_SIZE>();
 }
 
 


diff  --git a/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp 
b/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
index 15f67cea1ea98..2b131a2784925 100644
--- a/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
+++ b/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
@@ -435,8 +435,9 @@ struct EnvironmentVariables {
   int MaxTeamsDefault;
 };
 
+template 
 static constexpr const llvm::omp::GV &getGridValue() {
-  return llvm::omp::AMDGPUGridValues;
+  return llvm::omp::getAMDGPUGridValues();
 }
 
 /// Class containing all the device information
@@ -507,10 +508,24 @@ class RTLDeviceInfoTy {
   static const unsigned HardTeamLimit =
   

[PATCH] D108708: [openmp][amdgpu] Initial gfx10 offloading implementation

[Jon Chesterfield 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 rG78f92c38101f: [openmp][amdgpu] Initial gfx10 offloading 
implementation (authored by JonChesterfield).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108708

Files:
  clang/lib/Basic/Targets/AMDGPU.h
  llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h
  openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt
  openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h
  openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

Index: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
===
--- openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
+++ openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
@@ -435,8 +435,9 @@
   int MaxTeamsDefault;
 };
 
+template 
 static constexpr const llvm::omp::GV &getGridValue() {
-  return llvm::omp::AMDGPUGridValues;
+  return llvm::omp::getAMDGPUGridValues();
 }
 
 /// Class containing all the device information
@@ -507,10 +508,24 @@
   static const unsigned HardTeamLimit =
   (1 << 16) - 1; // 64K needed to fit in uint16
   static const int DefaultNumTeams = 128;
-  static const int Max_Teams = getGridValue().GV_Max_Teams;
-  static const int Warp_Size = getGridValue().GV_Warp_Size;
-  static const int Max_WG_Size = getGridValue().GV_Max_WG_Size;
-  static const int Default_WG_Size = getGridValue().GV_Default_WG_Size;
+
+  // These need to be per-device since different devices can have different
+  // wave sizes, but are currently the same number for each so that refactor
+  // can be postponed.
+  static_assert(getGridValue<32>().GV_Max_Teams ==
+getGridValue<64>().GV_Max_Teams,
+"");
+  static const int Max_Teams = getGridValue<64>().GV_Max_Teams;
+
+  static_assert(getGridValue<32>().GV_Max_WG_Size ==
+getGridValue<64>().GV_Max_WG_Size,
+"");
+  static const int Max_WG_Size = getGridValue<64>().GV_Max_WG_Size;
+
+  static_assert(getGridValue<32>().GV_Default_WG_Size ==
+getGridValue<64>().GV_Default_WG_Size,
+"");
+  static const int Default_WG_Size = getGridValue<64>().GV_Default_WG_Size;
 
   using MemcpyFunc = hsa_status_t (*)(hsa_signal_t, void *, const void *,
   size_t size, hsa_agent_t,
@@ -1059,8 +1074,9 @@
 DP("Queried wavefront size: %d\n", wavefront_size);
 DeviceInfo.WarpSize[device_id] = wavefront_size;
   } else {
-DP("Default wavefront size: %d\n", getGridValue().GV_Warp_Size);
-DeviceInfo.WarpSize[device_id] = getGridValue().GV_Warp_Size;
+// TODO: Burn the wavefront size into the code object
+DP("Warning: Unknown wavefront size, assuming 64\n");
+DeviceInfo.WarpSize[device_id] = 64;
   }
 
   // Adjust teams to the env variables
@@ -1885,9 +1901,10 @@
   int WorkgroupSize;
   int GridSize;
 };
-launchVals getLaunchVals(EnvironmentVariables Env, int ConstWGSize,
- int ExecutionMode, int num_teams, int thread_limit,
- uint64_t loop_tripcount, int DeviceNumTeams) {
+launchVals getLaunchVals(int WarpSize, EnvironmentVariables Env,
+ int ConstWGSize, int ExecutionMode, int num_teams,
+ int thread_limit, uint64_t loop_tripcount,
+ int DeviceNumTeams) {
 
   int threadsPerGroup = RTLDeviceInfoTy::Default_WG_Size;
   int num_groups = 0;
@@ -1900,7 +1917,7 @@
   if (print_kernel_trace & STARTUP_DETAILS) {
 DP("RTLDeviceInfoTy::Max_Teams: %d\n", RTLDeviceInfoTy::Max_Teams);
 DP("Max_Teams: %d\n", Max_Teams);
-DP("RTLDeviceInfoTy::Warp_Size: %d\n", RTLDeviceInfoTy::Warp_Size);
+DP("RTLDeviceInfoTy::Warp_Size: %d\n", WarpSize);
 DP("RTLDeviceInfoTy::Max_WG_Size: %d\n", RTLDeviceInfoTy::Max_WG_Size);
 DP("RTLDeviceInfoTy::Default_WG_Size: %d\n",
RTLDeviceInfoTy::Default_WG_Size);
@@ -1913,8 +1930,8 @@
 threadsPerGroup = thread_limit;
 DP("Setting threads per block to requested %d\n", thread_limit);
 if (ExecutionMode == GENERIC) { // Add master warp for GENERIC
-  threadsPerGroup += RTLDeviceInfoTy::Warp_Size;
-  DP("Adding master wavefront: +%d threads\n", RTLDeviceInfoTy::Warp_Size);
+  threadsPerGroup += WarpSize;
+  DP("Adding master wavefront: +%d threads\n", WarpSize);
 }
 if (threadsPerGroup > RTLDeviceInfoTy::Max_WG_Size) { // limit to max
   threadsPerGroup = RTLDeviceInfoTy::Max_WG_Size;
@@ -1950,7 +1967,7 @@
   // So we only handle constant thread_limits.
   if (threadsPerGroup >
   RTLDeviceInfoTy::Default_WG_Size) //  256 < threadsPerGroup <= 1024
-// Should we round threadsPerGroup up to nearest RTLDeviceInfoTy::Warp_Size
+// Should we round threadsPerGroup up to nearest WarpSize
 // here?
 num_groups = (M

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3788-3790
+if (Line.First && Line.First->Next &&
+Line.First->isOneOf(Keywords.kw_internal, tok::kw_public,
+tok::kw_private, tok::kw_protected) &&

Nit: can you add a boolean above the outer `if` so that it can be used in the 
`if` below?

Also, `Line.First` is inconsistent with `Line.startsWith` elsewhere? I think 
the latter skips leading comments.


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

https://reviews.llvm.org/D108810

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

[clang] 6808856 - [analyzer] MallocOverflow should consider comparisons only preceding malloc

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2021-08-27T14:41:26+02:00
New Revision: 68088563fbadba92a153cbe03c1586033b19322d

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

LOG: [analyzer] MallocOverflow should consider comparisons only preceding malloc

MallocOverflow works in two phases:

1) Collects suspicious malloc calls, whose argument is a multiplication
2) Filters the aggregated list of suspicious malloc calls by iterating
   over the BasicBlocks of the CFG looking for comparison binary
   operators over the variable constituting in any suspicious malloc.

Consequently, it suppressed true-positive cases when the comparison
check was after the malloc call.
In this patch the checker will consider the relative position of the
relation check to the malloc call.

E.g.:

```lang=C++
void *check_after_malloc(int n, int x) {
  int *p = NULL;
  if (x == 42)
p = malloc(n * sizeof(int)); // Previously **no** warning, now it
 // warns about this.

  // The check is after the allocation!
  if (n > 10) {
// Do something conditionally.
  }
  return p;
}
```

Reviewed By: martong

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

Added: 


Modified: 
clang/docs/analyzer/checkers.rst
clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
clang/test/Analysis/malloc-overflow.c

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index dc8698b8f0c8a..858c8e1303e82 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2190,10 +2190,6 @@ Limitations:
not tighten the domain to prevent the overflow in the subsequent
multiplication operation.
 
- - If the variable ``n`` participates in a comparison anywhere in the enclosing
-   function's scope, even after the ``malloc()``, the report will be still
-   suppressed.
-
  - It is an AST-based checker, thus it does not make use of the
path-sensitive taint-analysis.
 

diff  --git 
a/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
index e31630f63b5ac..7f1d0ac354f9b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
@@ -32,12 +32,14 @@ using llvm::APSInt;
 
 namespace {
 struct MallocOverflowCheck {
+  const CallExpr *call;
   const BinaryOperator *mulop;
   const Expr *variable;
   APSInt maxVal;
 
-  MallocOverflowCheck(const BinaryOperator *m, const Expr *v, APSInt val)
-  : mulop(m), variable(v), maxVal(std::move(val)) {}
+  MallocOverflowCheck(const CallExpr *call, const BinaryOperator *m,
+  const Expr *v, APSInt val)
+  : call(call), mulop(m), variable(v), maxVal(std::move(val)) {}
 };
 
 class MallocOverflowSecurityChecker : public Checker {
@@ -46,8 +48,8 @@ class MallocOverflowSecurityChecker : public 
Checker {
 BugReporter &BR) const;
 
   void CheckMallocArgument(
-SmallVectorImpl &PossibleMallocOverflows,
-const Expr *TheArgument, ASTContext &Context) const;
+  SmallVectorImpl &PossibleMallocOverflows,
+  const CallExpr *TheCall, ASTContext &Context) const;
 
   void OutputPossibleOverflows(
 SmallVectorImpl &PossibleMallocOverflows,
@@ -62,16 +64,15 @@ static inline bool EvaluatesToZero(APSInt &Val, 
BinaryOperatorKind op) {
 }
 
 void MallocOverflowSecurityChecker::CheckMallocArgument(
-  SmallVectorImpl &PossibleMallocOverflows,
-  const Expr *TheArgument,
-  ASTContext &Context) const {
+SmallVectorImpl &PossibleMallocOverflows,
+const CallExpr *TheCall, ASTContext &Context) const {
 
   /* Look for a linear combination with a single variable, and at least
one multiplication.
Reject anything that applies to the variable: an explicit cast,
conditional expression, an operation that could reduce the range
of the result, or anything too complicated :-).  */
-  const Expr *e = TheArgument;
+  const Expr *e = TheCall->getArg(0);
   const BinaryOperator * mulop = nullptr;
   APSInt maxVal;
 
@@ -115,9 +116,8 @@ void MallocOverflowSecurityChecker::CheckMallocArgument(
   // the data so when the body of the function is completely available
   // we can check for comparisons.
 
-  // TODO: Could push this into the innermost scope where 'e' is
-  // defined, rather than the whole function.
-  PossibleMallocOverflows.push_back(MallocOverflowCheck(mulop, e, maxVal));
+  PossibleMallocOverflows.push_back(
+  MallocOverflowCheck(TheCall, mulop, e, maxVal));
 }
 
 namespace {
@@ -158,12 +158,15 @@ class CheckOverflowOps :
 }
 
 void CheckExpr(const Expr *E_p) {
-  auto PredTrue = [](const MallocOverflowCh

[PATCH] D107804: [analyzer] MallocOverflow should consider comparisons only preceding malloc

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG68088563fbad: [analyzer] MallocOverflow should consider 
comparisons only preceding malloc (authored by steakhal).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107804

Files:
  clang/docs/analyzer/checkers.rst
  clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
  clang/test/Analysis/malloc-overflow.c

Index: clang/test/Analysis/malloc-overflow.c
===
--- clang/test/Analysis/malloc-overflow.c
+++ clang/test/Analysis/malloc-overflow.c
@@ -111,3 +111,40 @@
 return NULL;
   return malloc(n * sizeof(int));  // expected-warning {{the computation of the size of the memory allocation may overflow}}
 }
+
+void *check_before_malloc(int n, int x) {
+  int *p = NULL;
+  if (n > 10)
+return NULL;
+  if (x == 42)
+p = malloc(n * sizeof(int)); // no-warning, the check precedes the allocation
+
+  // Do some other stuff, e.g. initialize the memory.
+  return p;
+}
+
+void *check_after_malloc(int n, int x) {
+  int *p = NULL;
+  if (x == 42)
+p = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}}
+
+  // The check is after the allocation!
+  if (n > 10) {
+// Do something conditionally.
+  }
+  return p;
+}
+
+#define GREATER_THAN(lhs, rhs) (lhs > rhs)
+void *check_after_malloc_using_macros(int n, int x) {
+  int *p = NULL;
+  if (x == 42)
+p = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}}
+
+  if (GREATER_THAN(n, 10))
+return NULL;
+
+  // Do some other stuff, e.g. initialize the memory.
+  return p;
+}
+#undef GREATER_THAN
Index: clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
@@ -32,12 +32,14 @@
 
 namespace {
 struct MallocOverflowCheck {
+  const CallExpr *call;
   const BinaryOperator *mulop;
   const Expr *variable;
   APSInt maxVal;
 
-  MallocOverflowCheck(const BinaryOperator *m, const Expr *v, APSInt val)
-  : mulop(m), variable(v), maxVal(std::move(val)) {}
+  MallocOverflowCheck(const CallExpr *call, const BinaryOperator *m,
+  const Expr *v, APSInt val)
+  : call(call), mulop(m), variable(v), maxVal(std::move(val)) {}
 };
 
 class MallocOverflowSecurityChecker : public Checker {
@@ -46,8 +48,8 @@
 BugReporter &BR) const;
 
   void CheckMallocArgument(
-SmallVectorImpl &PossibleMallocOverflows,
-const Expr *TheArgument, ASTContext &Context) const;
+  SmallVectorImpl &PossibleMallocOverflows,
+  const CallExpr *TheCall, ASTContext &Context) const;
 
   void OutputPossibleOverflows(
 SmallVectorImpl &PossibleMallocOverflows,
@@ -62,16 +64,15 @@
 }
 
 void MallocOverflowSecurityChecker::CheckMallocArgument(
-  SmallVectorImpl &PossibleMallocOverflows,
-  const Expr *TheArgument,
-  ASTContext &Context) const {
+SmallVectorImpl &PossibleMallocOverflows,
+const CallExpr *TheCall, ASTContext &Context) const {
 
   /* Look for a linear combination with a single variable, and at least
one multiplication.
Reject anything that applies to the variable: an explicit cast,
conditional expression, an operation that could reduce the range
of the result, or anything too complicated :-).  */
-  const Expr *e = TheArgument;
+  const Expr *e = TheCall->getArg(0);
   const BinaryOperator * mulop = nullptr;
   APSInt maxVal;
 
@@ -115,9 +116,8 @@
   // the data so when the body of the function is completely available
   // we can check for comparisons.
 
-  // TODO: Could push this into the innermost scope where 'e' is
-  // defined, rather than the whole function.
-  PossibleMallocOverflows.push_back(MallocOverflowCheck(mulop, e, maxVal));
+  PossibleMallocOverflows.push_back(
+  MallocOverflowCheck(TheCall, mulop, e, maxVal));
 }
 
 namespace {
@@ -158,12 +158,15 @@
 }
 
 void CheckExpr(const Expr *E_p) {
-  auto PredTrue = [](const MallocOverflowCheck &) { return true; };
   const Expr *E = E_p->IgnoreParenImpCasts();
+  const auto PrecedesMalloc = [E, this](const MallocOverflowCheck &c) {
+return Context.getSourceManager().isBeforeInTranslationUnit(
+E->getExprLoc(), c.call->getExprLoc());
+  };
   if (const DeclRefExpr *DR = dyn_cast(E))
-Erase(DR, PredTrue);
+Erase(DR, PrecedesMalloc);
   else if (const auto *ME = dyn_cast(E)) {
-Erase(ME, PredTrue);
+Erase(ME, PrecedesMalloc);
   }
 }
 
@@ -322,7 +325,7 @@
 
   if (F

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

Ok, well, the reason I proposed this patch in the first place was that I have 
been working on a `.clang-format` schema (https://json-schema.org/) :) and I 
spotted the inconsistency. I checked, and clang-format reports an error if we 
give a negative value to an option expecting an unsigned. In the schema I am 
able to specify a minimum and I think it's appropriate to give the information 
to the user that it expect a positive/unsigned integer, what do you think?

Also, interesting question, how do we want to handle plural, as the formulation 
'List of Types'  introduces it. If we do it manually, it won't scale. We could 
include a dependency in python to something like inflect 
. I'll upload a new patch soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 369081.
FederAndInk added a comment.

Use yaml type style for clang-format documentation (`String`, `Integer`, `List 
of Strings`, ...) instead of c++ types.

Fix typo in clang/Format/Format.h

Regenarate ClangFormatStyleOptions.rst


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/include/clang/Format/Format.h

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3083,7 +3083,7 @@
 /// ForEach and If macros. This is useful in projects where ForEach/If
 /// macros are treated as function calls instead of control statements.
 /// ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
-/// backward compatability.
+/// backward compatibility.
 /// \code
 ///void f() {
 ///  Q_FOREACH(...) {
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -18,6 +18,30 @@
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def plural(word):
+  plurals = {
+'IncludeCategory': 'IncludeCategories'
+  }
+
+  return plurals.get(word, word + 's')
+
+def to_yaml_type(typestr):
+  typestr = str(typestr)
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+  
+  [subtype, napplied] = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + plural(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +64,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +109,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += indent('\nPossible values:\n\n', 2)
 s += indent('\n'.join(map(str, self.values)),2)
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -125,7 +125,7 @@
 the configuration (without a prefix: ``Auto``).
 
 
-**BasedOnStyle** (``string``)
+**BasedOnStyle** (``String``)
   The style used for all options not specifically set in the configuration.
 
   This option is supported only in the :program:`clang-format` configuration
@@ -166,7 +166,7 @@
 
 .. START_FORMAT_STYLE_OPTIONS
 
-**AccessModifierOffset** (``int``)
+**AccessModifierOffset** (``Integer``)
   The extra indent or outdent of access modifiers, e.g. ``public:``.
 
 **AlignAfterOpenBracket** (``BracketAlignmentStyle``)
@@ -619,7 +619,7 @@
 
 
 
-**AlignTrailingComments** (``bool``)
+**AlignTrailingComments** (``Boolean``)
   If ``true``, aligns trailing comments.
 
   .. code-block:: c++
@@ -628,7 +628,7 @@
 int a; // My comment a  vs. int a; // My comment a
 int b = 2; // comment  bint b = 2; // comment about b
 
-**AllowAllArgumentsOnNextLine** (``bool``)
+**AllowAllArgumentsOnNextLine** (``Boolean``)
   If a function call or braced initializer list doesn't fit on a
   line, allow putting all arguments onto the next line, even if
   ``BinPackArguments`` is ``false``.
@@ -645,7 +645,7 @@
  c,
  d);
 
-**AllowAllConstructorInitializersOnNextLine** (``bool``)
+**AllowAllConstructorInitializersOnNextLine** (``Boolean``)
   If a constructor definition with a member initializer list doesn't
   fit on a single line, allow putting all member initializers onto the next
   line, if ```ConstructorInitializerAllOnOneLineOrOnePerLine``` is true.
@@ -663,7 +663,7 @@
 member0(0),
 member1(2) {}
 
-**AllowAllParametersOfDeclarationOnNextLine** (``bool``)
+**AllowAllParametersOfDeclarationOnNextLine** (``Boolean``)
   If the function declaration doesn't fit on a line,
   allow putting all parameters of a function declaration onto
   the next line even if ``BinPackParameters`

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

For now, I handle plural manually, but it can be changed, I also kept Unsigned, 
what are your thoughts?

Thanks for being so kind and responsive, it's really great to work on that :) 
as it is my first contribution to clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

LGTM, thanks for adding that and fixing the spelling mistake, let the others 
have time to chip in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 369082.
MyDeveloperDay added a comment.

Handle comments before the enum


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

https://reviews.llvm.org/D108810

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -402,7 +402,9 @@
 }
 
 TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
-  verifyFormat("public enum var {\n"
+  // AfterEnum is true by default.
+  verifyFormat("public enum var\n"
+   "{\n"
"none,\n"
"@string,\n"
"bool,\n"
@@ -1099,5 +1101,218 @@
getGoogleStyle(FormatStyle::LK_Cpp));
 }
 
+TEST_F(FormatTestCSharp, CSharpAfterEnum) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterEnum = false;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("internal enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("public enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("protected enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("private enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterEnum = true;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("internal enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("public enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("protected enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("private enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("/* Foo */ private enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("/* Foo */ /* Bar */ private enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+}
+
+TEST_F(FormatTestCSharp, CSharpAfterClass) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = false;
+
+  verifyFormat("class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("public class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("protected class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("private class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+
+  verifyFormat("interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("public interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("protected interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("private interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterClass = true;
+
+  verifyFormat("class MyClass\n"
+   "{\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+ 

[PATCH] D105269: [X86] AVX512FP16 instructions enabling 6/6

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added inline comments.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:47419
+   : X86ISD::VFCMADDC;
+  // FIXME: How we handle when FMF of FADD is different from CFMUL's?
+  CFmul = DAG.getNode(newOp, SDLoc(N), CVT, FAddOp1, CFmul.getOperand(0),

Sorry, I don't understand the comments. What does FMF mean?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105269

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

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

LGTM.




Comment at: clang/lib/Format/TokenAnnotator.cpp:3799
+if (Style.BraceWrapping.AfterEnum) {
+  if (Line.startsWith(tok::kw_enum) ||
+  Line.startsWith(tok::kw_typedef, tok::kw_enum))

Nit: I think it would be a little more efficient to call `FirstNonComment->is` 
instead.


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

https://reviews.llvm.org/D108810

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

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Nice work!




Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:361
+  while (!SCtx->inTopFrame()) {
+auto p = FramesModifying.insert(SCtx);
+if (!p.second)

Why don't we add the stack frame here to `FramesModifyingCalculated` as well? 
If we are in a nested stack frame and we step back to a parent then it would be 
redundant to do the calculation again for the parent.

Ahh, to be honest, I think the idea of having both `FramesModifying` and 
`FramesModifyingCalculated` separated is prone to errors.
Why don't we have a simple map with the boolean true 
meaning that the frame is modifying? And if the frame is not in the map that 
means it had never been calculated before.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:370
+  while (N && !N->getLocationAs())
+N = N->getFirstSucc();
+  return N;

Szelethus wrote:
> NoQ wrote:
> > This is the right successor because we're in a heavily trimmed exploded 
> > graph, right?
> Should be, yes.
Would it make sense to assert that there are no more than one successor ?



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:386
+
+  while (CurrN) {
+// Found a new inlined call.

Using a `for` could simplify this loop and would eliminate the need to have a 
redundant loop variable bump both at line 420 and at 393.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108695

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

[PATCH] D107804: [analyzer] MallocOverflow should consider comparisons only preceding malloc

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Probably it is too late, but the title of this change is misleading. I think is 
should be either `MallocOverflow should consider comparisons AFTER malloc` or 
`MallocOverflow should NOT consider comparisons only preceding malloc`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107804

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D108765#2969036 , @FederAndInk 
wrote:

> For now, I handle plural manually, but it can be changed, I also kept 
> Unsigned, what are your thoughts?



  I think thats ok for now, did you try building the file with sphinx-build, I 
run
  
  `/usr/bin/sphinx-build -n ./docs ./html`
  
  Then go and look at the html

> Thanks for being so kind and responsive, it's really great to work on that :) 
> as it is my first contribution to clang.



  That is how it should be right ;-).
  
  I assume you don't have commit access, we'll need your name and email address 
if you want one of us to land it, so we can accredit you with the contribution, 
but if you think you might like to work on some other things, it might be worth 
get ting the  "getting commit access" process started.
  
  I think its always good to wait to give others some chance to comment before 
we commit, as we are all in different timezones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D105269: [X86] AVX512FP16 instructions enabling 6/6

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:47419
+   : X86ISD::VFCMADDC;
+  // FIXME: How we handle when FMF of FADD is different from CFMUL's?
+  CFmul = DAG.getNode(newOp, SDLoc(N), CVT, FAddOp1, CFmul.getOperand(0),

LuoYuanke wrote:
> Sorry, I don't understand the comments. What does FMF mean?
fast math flags?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105269

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

[clang] 8a780a2 - [clang-format] Group options that pack constructor initializers

[via cfe-commits]

Author: owenca
Date: 2021-08-27T06:27:46-07:00
New Revision: 8a780a2f18c590e27e51a2ab3cc81b481c42b42a

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

LOG: [clang-format] Group options that pack constructor initializers

Add a new option PackConstructorInitializers and deprecate the
related options ConstructorInitializerAllOnOneLineOrOnePerLine and
AllowAllConstructorInitializersOnNextLine. Below is the mapping:

PackConstructorInitializers  ConstructorInitializer... AllowAll...
Never-  -
BinPackfalse-
CurrentLinetrue   false
NextLine   true   true

The option value Never fixes PR50549 by always placing each
constructor initializer on its own line.

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

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/Format.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index d8ac58734dc4a..677d498a1e57a 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -646,22 +646,8 @@ the configuration (without a prefix: ``Auto``).
  d);
 
 **AllowAllConstructorInitializersOnNextLine** (``bool``)
-  If a constructor definition with a member initializer list doesn't
-  fit on a single line, allow putting all member initializers onto the next
-  line, if ```ConstructorInitializerAllOnOneLineOrOnePerLine``` is true.
-  Note that this parameter has no effect if
-  ```ConstructorInitializerAllOnOneLineOrOnePerLine``` is false.
-
-  .. code-block:: c++
-
-true:
-MyClass::MyClass() :
-member0(0), member1(2) {}
-
-false:
-MyClass::MyClass() :
-member0(0),
-member1(2) {}
+  This option is **deprecated**. See ``NextLine`` of
+  ``PackConstructorInitializers``.
 
 **AllowAllParametersOfDeclarationOnNextLine** (``bool``)
   If the function declaration doesn't fit on a line,
@@ -2002,7 +1988,7 @@ the configuration (without a prefix: ``Auto``).
  SecondValueVeryVeryVeryVeryLong;
 
 **BreakConstructorInitializers** (``BreakConstructorInitializersStyle``)
-  The constructor initializers style to use.
+  The break constructor initializers style to use.
 
   Possible values:
 
@@ -2140,23 +2126,8 @@ the configuration (without a prefix: ``Auto``).
 }}}
 
 **ConstructorInitializerAllOnOneLineOrOnePerLine** (``bool``)
-  If the constructor initializers don't fit on a line, put each
-  initializer on its own line.
-
-  .. code-block:: c++
-
-true:
-SomeClass::Constructor()
-: (), (), 
(a) {
-  return 0;
-}
-
-false:
-SomeClass::Constructor()
-: (), (),
-  (a) {
-  return 0;
-}
+  This option is **deprecated**. See ``CurrentLine`` of
+  ``PackConstructorInitializers``.
 
 **ConstructorInitializerIndentWidth** (``unsigned``)
   The number of characters to use for indentation of constructor
@@ -3143,6 +3114,60 @@ the configuration (without a prefix: ``Auto``).
  # define BAR
  #endif
 
+**PackConstructorInitializers** (``PackConstructorInitializersStyle``)
+  The pack constructor initializers style to use.
+
+  Possible values:
+
+  * ``PCIS_Never`` (in configuration: ``Never``)
+Always put each constructor initializer on its own line.
+
+.. code-block:: c++
+
+   Constructor()
+   : a(),
+ b()
+
+  * ``PCIS_BinPack`` (in configuration: ``BinPack``)
+Bin-pack constructor initializers.
+
+.. code-block:: c++
+
+   Constructor()
+   : (), (),
+ ()
+
+  * ``PCIS_CurrentLine`` (in configuration: ``CurrentLine``)
+Put all constructor initializers on the current line if they fit.
+Otherwise, put each one on its own line.
+
+.. code-block:: c++
+
+   Constructor() : a(), b()
+
+   Constructor()
+   : (),
+ (),
+ d()
+
+  * ``PCIS_NextLine`` (in configuration: ``NextLine``)
+Same as ``PCIS_CurrentLine`` except that if all constructor initializers
+do not fit on the current line, try to fit them on the next line.
+
+.. code-block:: c++
+
+   Constructor() : a(), b()
+
+   Constructor()
+   : aa

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a780a2f18c5: [clang-format] Group options that pack 
constructor initializers (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108752

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5960,7 +5960,26 @@
"  at() {}");
 
   FormatStyle OnePerLine = getLLVMStyle();
-  OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  OnePerLine.PackConstructorInitializers = FormatStyle::PCIS_Never;
+  verifyFormat("MyClass::MyClass()\n"
+   ": a(a),\n"
+   "  b(b),\n"
+   "  c(c) {}",
+   OnePerLine);
+  verifyFormat("MyClass::MyClass()\n"
+   ": a(a), // comment\n"
+   "  b(b),\n"
+   "  c(c) {}",
+   OnePerLine);
+  verifyFormat("MyClass::MyClass(int a)\n"
+   ": b(a),  // comment\n"
+   "  c(a + 1) { // lined up\n"
+   "}",
+   OnePerLine);
+  verifyFormat("Constructor()\n"
+   ": a(b, b, b) {}",
+   OnePerLine);
+  OnePerLine.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
   OnePerLine.AllowAllParametersOfDeclarationOnNextLine = false;
   verifyFormat("SomeClass::Constructor()\n"
": a(aa),\n"
@@ -6013,8 +6032,6 @@
   FormatStyle Style = getLLVMStyle();
   Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
   Style.ColumnLimit = 60;
-  Style.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
-  Style.AllowAllConstructorInitializersOnNextLine = true;
   Style.BinPackParameters = false;
 
   for (int i = 0; i < 4; ++i) {
@@ -6022,14 +6039,14 @@
 Style.AllowAllParametersOfDeclarationOnNextLine = i & 1;
 Style.AllowAllArgumentsOnNextLine = i & 2;
 
-Style.AllowAllConstructorInitializersOnNextLine = true;
+Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
 Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
 verifyFormat("Constructor()\n"
  ": (a), b(b) {}",
  Style);
 verifyFormat("Constructor() : a(a), b(b) {}", Style);
 
-Style.AllowAllConstructorInitializersOnNextLine = false;
+Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
 verifyFormat("Constructor()\n"
  ": (a)\n"
  ", b(b) {}",
@@ -6037,24 +6054,24 @@
 verifyFormat("Constructor() : a(a), b(b) {}", Style);
 
 Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
-Style.AllowAllConstructorInitializersOnNextLine = true;
+Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
 verifyFormat("Constructor()\n"
  ": (a), b(b) {}",
  Style);
 
-Style.AllowAllConstructorInitializersOnNextLine = false;
+Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
 verifyFormat("Constructor()\n"
  ": (a),\n"
  "  b(b) {}",
  Style);
 
 Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon;
-Style.AllowAllConstructorInitializersOnNextLine = true;
+Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
 verifyFormat("Constructor() :\n"
  "aa(a), b(b) {}",
  Style);
 
-Style.AllowAllConstructorInitializersOnNextLine = false;
+Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
 verifyFormat("Constructor() :\n"
  "aa(a),\n"
  "b(b) {}",
@@ -6066,14 +6083,13 @@
   // BreakConstructorInitializers modes
   Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
   Style.AllowAllParametersOfDeclarationOnNextLine = true;
-  Style.AllowAllConstructorInitializersOnNextLine = false;
   verifyFormat("SomeClassWithALongName::Constructor(\n"
"int , int b)\n"
": (a)\n"
", b(b) {}",
Style);
 
-  Style.AllowAllConstr

[PATCH] D105269: [X86] AVX512FP16 instructions enabling 6/6

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added inline comments.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:47419
+   : X86ISD::VFCMADDC;
+  // FIXME: How we handle when FMF of FADD is different from CFMUL's?
+  CFmul = DAG.getNode(newOp, SDLoc(N), CVT, FAddOp1, CFmul.getOperand(0),

RKSimon wrote:
> LuoYuanke wrote:
> > Sorry, I don't understand the comments. What does FMF mean?
> fast math flags?
I understand now. Thanks, Simon. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105269

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

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:675
+  /// retrieved from a CallEnter is the *caller's* stack frame! The inlined
+  /// function's stack should be retrieved from \p CallExitBeginN.
+  virtual bool wasModifiedInFunction(const ExplodedNode *CallEnterN,

You have no such parameter. But I guess we can get that from either of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108695

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

> look at the html

Done, thanks

> I assume you don't have commit access, we'll need your name and email address

Here it is: Ludovic Jozeau  it should also be on my 
profile

> but if you think you might like to work on some other things, it might be 
> worth get ting the  "getting commit access" process started.

Yeah, after reading 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access, I think 
I'll wait until I do another contribution. :)

> I think its always good to wait to give others some chance to comment before 
> we commit, as we are all in different timezones.

Ok, that's fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D108753: [analyzer] MallocChecker: Add notes from NoOwnershipChangeVisitor only when a function "intents", but doesn't change ownership, enable by default

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:802
+// TODO: Operator delete is hardly the only deallocator -- Can we reuse
+// isFreeingCall() or something thats already here?
+auto Deallocations = match(

Sounds like a good idea to reuse `isFreeingCall`. 

But interestingly `delete` is not listed there! Nor `new` in 
`AllocatingMemFnMap`. So, perhaps we need yet another abstraction. Maybe a 
function that iterates over `FreeingMemFnMap` collects the functions and then 
adds `delete` as well?



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:806
+ ), *FD->getBody(), ACtx);
+// TODO: Ownership my change with an attempt to store the allocated memory.
+return !Deallocations.empty();

Good point!



Comment at: clang/test/Analysis/NewDeleteLeaks.cpp:23
 
+// TODO: AST analysis of sink would reveal that it doesn't indent to free the
+// allocated memory, but in this instance, its also the only function with

intent

BTW, why is this TODO here? It is obvious that `sink` does not have a `delete`, 
so we don't expect any warning. Or am I missing something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108753

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

[PATCH] D105269: [X86] AVX512FP16 instructions enabling 6/6

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:47419
+   : X86ISD::VFCMADDC;
+  // FIXME: How we handle when FMF of FADD is different from CFMUL's?
+  CFmul = DAG.getNode(newOp, SDLoc(N), CVT, FAddOp1, CFmul.getOperand(0),

LuoYuanke wrote:
> RKSimon wrote:
> > LuoYuanke wrote:
> > > Sorry, I don't understand the comments. What does FMF mean?
> > fast math flags?
> I understand now. Thanks, Simon. :)
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105269

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

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a reviewer: efriedma.
spatel added a comment.

I'm not familiar with this library, and I haven't looked at current state of 
how we enable/map optional libs in a while...
We definitely want to avoid adding another target option/debug flag, and if we 
can avoid relying on a function parameter too, that would be even better.
Ie, the "afn" fast-math-flag (possibly in combination with some other IR- or 
node-level flags) seems like it should be enough to allow this 
transform/lowering. 
Scanning the earlier review comments, there was some concern about the 
semantics wrt errno. If we need to adjust the "afn" definition, it's probably 
fine. There haven't been many uses of that flag AFAIK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101759

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

[PATCH] D106191: [clang] Option control afn flag

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

FWIW i think this part is fine in principle.
Not sure about errno.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106191

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

[PATCH] D108818: [clang] Add a -canonical-prefixes option

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added reviewers: hans, rnk.
Herald added a subscriber: dang.
thakis requested review of this revision.

In https://reviews.llvm.org/D47480 I complained that there's no positive
form of this flag, so let's add one :)

https://gcc.gnu.org/PR29931 also has a pending patch to add the positive
form to gcc (but there's admittedly not a lot of movement on that bug).

This doesn't change any defaults.


https://reviews.llvm.org/D108818

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/no-canonical-prefixes.c
  clang/tools/driver/driver.cpp


Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -416,10 +416,10 @@
 // Skip end-of-line response file markers
 if (Args[i] == nullptr)
   continue;
-if (StringRef(Args[i]) == "-no-canonical-prefixes") {
+if (StringRef(Args[i]) == "-canonical-prefixes")
+  CanonicalPrefixes = true;
+else if (StringRef(Args[i]) == "-no-canonical-prefixes")
   CanonicalPrefixes = false;
-  break;
-}
   }
 
   // Handle CL and _CL_ which permits additional command line options to be
Index: clang/test/Driver/no-canonical-prefixes.c
===
--- clang/test/Driver/no-canonical-prefixes.c
+++ clang/test/Driver/no-canonical-prefixes.c
@@ -10,8 +10,20 @@
 // RUN: rm -f %t.fake
 // RUN: ln -sf %t.real %t.fake
 // RUN: cd %t.fake
-// RUN: ./test-clang -v -S %s 2>&1 | FileCheck --check-prefix=CANONICAL %s
-// RUN: ./test-clang -v -S %s -no-canonical-prefixes 2>&1 | FileCheck 
--check-prefix=NON-CANONICAL %s
+// RUN: ./test-clang -v -S %s 2>&1 \
+// RUN: | FileCheck --check-prefix=CANONICAL %s
+// RUN: ./test-clang -v -S %s 2>&1 \
+// RUN: -no-canonical-prefixes \
+// RUN: | FileCheck --check-prefix=NON-CANONICAL %s
+// RUN: ./test-clang -v -S %s 2>&1 \
+// RUN: -no-canonical-prefixes \
+// RUN: -canonical-prefixes \
+// RUN: | FileCheck --check-prefix=CANONICAL %s
+// RUN: ./test-clang -v -S %s 2>&1 \
+// RUN: -no-canonical-prefixes \
+// RUN: -canonical-prefixes \
+// RUN: -no-canonical-prefixes \
+// RUN: | FileCheck --check-prefix=NON-CANONICAL %s
 //
 // FIXME: This should really be '.real'.
 // CANONICAL: InstalledDir: {{.*}}.fake
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1091,7 +1091,8 @@
   // Silence driver warnings if requested
   Diags.setIgnoreAllWarnings(Args.hasArg(options::OPT_w));
 
-  // -no-canonical-prefixes is used very early in main.
+  // -canonical-prefixes, -no-canonical-prefixes are used very early in main.
+  Args.ClaimAllArgs(options::OPT_canonical_prefixes);
   Args.ClaimAllArgs(options::OPT_no_canonical_prefixes);
 
   // f(no-)integated-cc1 is also used very early in main.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3676,8 +3676,10 @@
 def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
 def multiply__defined : Separate<["-"], "multiply_defined">;
 def mwarn_nonportable_cfstrings : Flag<["-"], "mwarn-nonportable-cfstrings">, 
Group;
+def canonical_prefixes : Flag<["-"], "canonical-prefixes">, Flags<[HelpHidden, 
CoreOption]>,
+  HelpText<"Use absolute paths for invoking subcommands (default)">;
 def no_canonical_prefixes : Flag<["-"], "no-canonical-prefixes">, 
Flags<[HelpHidden, CoreOption]>,
-  HelpText<"Use relative instead of canonical paths">;
+  HelpText<"Use relative paths for invoking subcommands">;
 def no_cpp_precomp : Flag<["-"], "no-cpp-precomp">, 
Group;
 def no_integrated_cpp : Flag<["-", "--"], "no-integrated-cpp">, 
Flags<[NoXarchOption]>;
 def no_pedantic : Flag<["-", "--"], "no-pedantic">, Group;


Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -416,10 +416,10 @@
 // Skip end-of-line response file markers
 if (Args[i] == nullptr)
   continue;
-if (StringRef(Args[i]) == "-no-canonical-prefixes") {
+if (StringRef(Args[i]) == "-canonical-prefixes")
+  CanonicalPrefixes = true;
+else if (StringRef(Args[i]) == "-no-canonical-prefixes")
   CanonicalPrefixes = false;
-  break;
-}
   }
 
   // Handle CL and _CL_ which permits additional command line options to be
Index: clang/test/Driver/no-canonical-prefixes.c
===
--- clang/test/Driver/no-canonical-prefixes.c
+++ clang/test/Driver/no-canonical-prefixes.c
@@ -10,8 +10,20 @@
 // RUN: rm -f %t.fake
 // RUN: ln -sf %t.real %t.fake
 // RUN: cd 

[PATCH] D108819: [MCParser][z/OS] Mark test as unsupported for the z/OS Target

[Fanbo Meng via Phabricator via cfe-commits]

fanbo-meng created this revision.
fanbo-meng requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Marking test as unsupported for the same reason as 
https://reviews.llvm.org/D105204


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108819

Files:
  clang/test/Driver/as-version.s


Index: clang/test/Driver/as-version.s
===
--- clang/test/Driver/as-version.s
+++ clang/test/Driver/as-version.s
@@ -1,5 +1,6 @@
 // Test version information.
 
+// UNSUPPORTED: -zos
 // RUN: %clang -Wa,--version -c -fintegrated-as %s -o /dev/null \
 // RUN:   | FileCheck --check-prefix=IAS %s
 // IAS: clang version


Index: clang/test/Driver/as-version.s
===
--- clang/test/Driver/as-version.s
+++ clang/test/Driver/as-version.s
@@ -1,5 +1,6 @@
 // Test version information.
 
+// UNSUPPORTED: -zos
 // RUN: %clang -Wa,--version -c -fintegrated-as %s -o /dev/null \
 // RUN:   | FileCheck --check-prefix=IAS %s
 // IAS: clang version
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108819: [MCParser][z/OS] Mark test as unsupported for the z/OS Target

[Abhina Sree via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108819

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

[PATCH] D104344: [modules] Track how headers are included by different modules.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

If my understanding is correct, this patch only changes how we track 
`HeaderFileInfo::{isImport,NumIncludes}` and makes sure to store the necessary 
info into AST files. Right?

Can you explain how values of these members can prevent Clang from importing 
the header and how this interacts with visibility?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104344

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

[PATCH] D108822: [python bindings] Add missing cursor kind in Clang Python's bindings.

[Colin Chargy via Phabricator via cfe-commits]

ColinChargy created this revision.
ColinChargy added a reviewer: rsmith.
ColinChargy added a project: clang.
Herald added a subscriber: arphaman.
ColinChargy requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.

Some of the values in CXCursorKind in clang\include\clang-c\Index.h (lines 
1706-2686) where not handled in clang\bindings\python\clang\cindex.py, leading 
to raise a Python exception where retrieving kind of such Cursors:

File "/usr/lib/python3/dist-packages/clang/cindex.py", line 650, in from_id
  raise ValueError('Unknown template argument kind %d' % id)
  ValueError: Unknown template argument kind 280

I hope this patch is OK.

I think we could, in a future patch, put these definitions in a single file to 
make sure both C and python always match; let me know if that does seem 
appealing.

Regards,
Colin Chargy


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108822

Files:
  clang/bindings/python/clang/cindex.py


Index: clang/bindings/python/clang/cindex.py
===
--- clang/bindings/python/clang/cindex.py
+++ clang/bindings/python/clang/cindex.py
@@ -1084,6 +1084,18 @@
 # Represents an @available(...) check.
 CursorKind.OBJC_AVAILABILITY_CHECK_EXPR = CursorKind(148)
 
+# Fixed point literal
+CursorKind.FIXED_POINT_LITERAL = CursorKind(149)
+
+# OpenMP 5.0 [2.1.4, Array Shaping].
+CursorKind.OMP_ARRAY_SHAPING_EXPR = CursorKind(150)
+
+# OpenMP 5.0 [2.1.6 Iterators]
+CursorKind.OMP_ITERATOR_EXPR = CursorKind(151)
+
+# OpenCL's addrspace_cast<> expression.
+CursorKind.CXX_ADDRSPACE_CAST_EXPR = CursorKind(152)
+
 
 # A statement whose specific kind is not exposed via this interface.
 #
@@ -1305,6 +1317,72 @@
 # OpenMP teams distribute directive.
 CursorKind.OMP_TEAMS_DISTRIBUTE_DIRECTIVE = CursorKind(271)
 
+# OpenMP teams distribute simd directive.
+CursorKind.OMP_TEAMS_DISTRIBUTE_SIMD_DIRECTIVE = CursorKind(272)
+
+# OpenMP teams distribute parallel for simd directive.
+CursorKind.OMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_SIMD_DIRECTIVE = CursorKind(273)
+
+# OpenMP teams distribute parallel for directive.
+CursorKind.OMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_DIRECTIVE = CursorKind(274)
+
+# OpenMP target teams directive.
+CursorKind.OMP_TARGET_TEAMS_DIRECTIVE = CursorKind(275)
+
+# OpenMP target teams distribute directive.
+CursorKind.OMP_TARGET_TEAMS_DISTRIBUTE_DIRECTIVE = CursorKind(276)
+
+# OpenMP target teams distribute parallel for directive.
+CursorKind.OMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_FOR_DIRECTIVE = CursorKind(277)
+
+# OpenMP target teams distribute parallel for simd directive.
+CursorKind.OMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_FOR_SIMD_DIRECTIVE = 
CursorKind(278)
+
+# OpenMP target teams distribute simd directive.
+CursorKind.OMP_TARGET_TEAMS_DISTRIBUTE_SIMD_DIRECTIVE = CursorKind(279)
+
+# C++2a std::bit_cast expression.
+CursorKind.BUILTIN_BIT_CAST_EXPR = CursorKind(280)
+
+# OpenMP master taskloop directive.
+CursorKind.OMP_MASTER_TASK_LOOP_DIRECTIVE = CursorKind(281)
+
+# OpenMP parallel master taskloop directive.
+CursorKind.OMP_PARALLEL_MASTER_TASK_LOOP_DIRECTIVE = CursorKind(282)
+
+# OpenMP master taskloop simd directive.
+CursorKind.OMP_MASTER_TASK_LOOP_SIMD_DIRECTIVE = CursorKind(283)
+
+# OpenMP parallel master taskloop simd directive.
+CursorKind.OMP_PARALLEL_MASTER_TASK_LOOP_SIMD_DIRECTIVE = CursorKind(284)
+
+# OpenMP parallel master directive.
+CursorKind.OMP_PARALLEL_MASTER_DIRECTIVE = CursorKind(285)
+
+# OpenMP depobj directive.
+CursorKind.OMP_DEPOBJ_DIRECTIVE = CursorKind(286)
+
+# OpenMP scan directive.
+CursorKind.OMP_SCAN_DIRECTIVE = CursorKind(287)
+
+# OpenMP tile directive.
+CursorKind.OMP_TILE_DIRECTIVE = CursorKind(288)
+
+# OpenMP canonical loop.
+CursorKind.OMP_CANONICAL_DIRECTIVE = CursorKind(289)
+
+# OpenMP interop directive.
+CursorKind.OMP_INTEROP_DIRECTIVE = CursorKind(290)
+
+# OpenMP dispatch directive.
+CursorKind.OMP_DISPATCH_DIRECTIVE = CursorKind(291)
+
+# OpenMP masked directive.
+CursorKind.OMP_MASKED_DIRECTIVE = CursorKind(292)
+
+# OpenMP unroll directive.
+CursorKind.OMP_UNROLL_DIRECTIVE = CursorKind(293)
+
 ###
 # Other Kinds
 


Index: clang/bindings/python/clang/cindex.py
===
--- clang/bindings/python/clang/cindex.py
+++ clang/bindings/python/clang/cindex.py
@@ -1084,6 +1084,18 @@
 # Represents an @available(...) check.
 CursorKind.OBJC_AVAILABILITY_CHECK_EXPR = CursorKind(148)
 
+# Fixed point literal
+CursorKind.FIXED_POINT_LITERAL = CursorKind(149)
+
+# OpenMP 5.0 [2.1.4, Array Shaping].
+CursorKind.OMP_ARRAY_SHAPING_EXPR = CursorKind(150)
+
+# OpenMP 5.0 [2.1.6 Iterators]
+CursorKind.OMP_ITERATOR_EXPR = CursorKind(151)
+
+# OpenCL's addrspace_cast<> expression.
+CursorKind.CXX_ADDRSPACE_CAST_EXPR = CursorKind(152)
+
 
 # A statement whose specific kind is not exposed via this interface.
 #
@@ -1305,6 +1317,72 @@
 # OpenMP teams

[PATCH] D104344: [modules] Track how headers are included by different modules.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

FYI, a clangd test is failing in CI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104344

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

[PATCH] D108823: [PowerPC] Mark splat immediate instructions as rematerializable

[Victor Huang via Phabricator via cfe-commits]

NeHuang created this revision.
NeHuang added reviewers: nemanjai, stefanp, lei, PowerPC.
NeHuang added a project: LLVM.
Herald added subscribers: shchenz, kbarton, hiraditya, qcolombet.
NeHuang requested review of this revision.

This patch marks splat immediate instructions `XXSPLTIDP` and `XXSPLTI32DX` as 
rematerializable to prevent MachineLICM from moving them out of loops.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108823

Files:
  llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/constant-pool.ll
  llvm/test/CodeGen/PowerPC/p10-spill-crun.ll
  llvm/test/CodeGen/PowerPC/p10-splatImm.ll

Index: llvm/test/CodeGen/PowerPC/p10-splatImm.ll
===
--- llvm/test/CodeGen/PowerPC/p10-splatImm.ll
+++ llvm/test/CodeGen/PowerPC/p10-splatImm.ll
@@ -249,7 +249,6 @@
 ; CHECK-LABEL: testFloatScalar:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:xxspltidp vs1, 1135290941
-; CHECK-NEXT:# kill: def $f1 killed $f1 killed $vsl1
 ; CHECK-NEXT:blr
 
 entry:
@@ -270,7 +269,6 @@
 ; CHECK-LABEL: testDoubleRepresentableScalar:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:xxspltidp vs1, 1135290941
-; CHECK-NEXT:# kill: def $f1 killed $f1 killed $vsl1
 ; CHECK-NEXT:blr
 
 entry:
Index: llvm/test/CodeGen/PowerPC/p10-spill-crun.ll
===
--- llvm/test/CodeGen/PowerPC/p10-spill-crun.ll
+++ llvm/test/CodeGen/PowerPC/p10-spill-crun.ll
@@ -35,8 +35,7 @@
 
 define dso_local void @P10_Spill_CR_UN(%2* %arg, %1* %arg1, i32 %arg2) local_unnamed_addr {
 ; CHECK-LABEL: P10_Spill_CR_UN:
-; CHECK: .localentry P10_Spill_CR_UN, 1
-; CHECK-NEXT:  # %bb.0: # %bb
+; CHECK:   # %bb.0: # %bb
 ; CHECK-NEXT:mflr r0
 ; CHECK-NEXT:mfcr r12
 ; CHECK-NEXT:std r0, 16(r1)
@@ -146,19 +145,19 @@
 ; CHECK-NEXT:# implicit-def: $r3
 ; CHECK-NEXT:  .LBB0_15: # %bb50
 ; CHECK-NEXT:li r4, 0
-; CHECK-NEXT:xxspltidp vs3, -1082130432
 ; CHECK-NEXT:extsh r9, r3
 ; CHECK-NEXT:extsw r6, r28
 ; CHECK-NEXT:li r5, 0
+; CHECK-NEXT:xxspltidp vs3, -1082130432
+; CHECK-NEXT:xxspltidp vs4, -1082130432
 ; CHECK-NEXT:std r30, 104(r1)
 ; CHECK-NEXT:std r29, 96(r1)
 ; CHECK-NEXT:li r7, 0
 ; CHECK-NEXT:li r8, 0
 ; CHECK-NEXT:li r10, 0
-; CHECK-NEXT:fmr f4, f3
-; CHECK-NEXT:xxlxor f1, f1, f1
 ; CHECK-NEXT:std r4, 152(r1)
 ; CHECK-NEXT:li r4, -1
+; CHECK-NEXT:xxlxor f1, f1, f1
 ; CHECK-NEXT:std r4, 112(r1)
 ; CHECK-NEXT:li r4, 1024
 ; CHECK-NEXT:bl call_4@notoc
@@ -304,19 +303,19 @@
 ; CHECK-BE-NEXT:# implicit-def: $r3
 ; CHECK-BE-NEXT:  .LBB0_15: # %bb50
 ; CHECK-BE-NEXT:li r4, 0
-; CHECK-BE-NEXT:xxspltidp vs3, -1082130432
 ; CHECK-BE-NEXT:extsh r9, r3
 ; CHECK-BE-NEXT:extsw r6, r28
 ; CHECK-BE-NEXT:li r5, 0
+; CHECK-BE-NEXT:xxspltidp vs3, -1082130432
+; CHECK-BE-NEXT:xxspltidp vs4, -1082130432
 ; CHECK-BE-NEXT:std r30, 120(r1)
 ; CHECK-BE-NEXT:std r29, 112(r1)
 ; CHECK-BE-NEXT:li r7, 0
 ; CHECK-BE-NEXT:li r8, 0
 ; CHECK-BE-NEXT:li r10, 0
-; CHECK-BE-NEXT:fmr f4, f3
-; CHECK-BE-NEXT:xxlxor f1, f1, f1
 ; CHECK-BE-NEXT:std r4, 168(r1)
 ; CHECK-BE-NEXT:li r4, -1
+; CHECK-BE-NEXT:xxlxor f1, f1, f1
 ; CHECK-BE-NEXT:std r4, 128(r1)
 ; CHECK-BE-NEXT:li r4, 1024
 ; CHECK-BE-NEXT:bl call_4
Index: llvm/test/CodeGen/PowerPC/constant-pool.ll
===
--- llvm/test/CodeGen/PowerPC/constant-pool.ll
+++ llvm/test/CodeGen/PowerPC/constant-pool.ll
@@ -364,15 +364,15 @@
 ; CHECK-NEXT:.cfi_def_cfa_offset 48
 ; CHECK-NEXT:.cfi_offset lr, 16
 ; CHECK-NEXT:.cfi_offset v31, -16
+; CHECK-NEXT:xxlxor f4, f4, f4
+; CHECK-NEXT:xxsplti32dx vs3, 0, 1074935889
 ; CHECK-NEXT:stxv vs63, 32(r1) # 16-byte Folded Spill
 ; CHECK-NEXT:xxsplti32dx vs63, 0, 1074935889
-; CHECK-NEXT:xxlxor f4, f4, f4
-; CHECK-NEXT:xxlor vs3, vs63, vs63
 ; CHECK-NEXT:xxsplti32dx vs3, 1, -343597384
 ; CHECK-NEXT:# kill: def $f3 killed $f3 killed $vsl3
 ; CHECK-NEXT:bl __gcc_qadd@notoc
-; CHECK-NEXT:xxlor vs3, vs63, vs63
 ; CHECK-NEXT:xxlxor f4, f4, f4
+; CHECK-NEXT:xxsplti32dx vs3, 0, 1074935889
 ; CHECK-NEXT:xxsplti32dx vs3, 1, -1719329096
 ; CHECK-NEXT:# kill: def $f3 killed $f3 killed $vsl3
 ; CHECK-NEXT:bl __gcc_qadd@notoc
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -1861,15 +1861,6 @@
 }
 
 let Predicates = [PrefixInstrs] in {
-  def XXSPLTIW : 8RR_DForm_IMM32_XT6<32, 3, (outs vsrc:$XT),
- (ins i32imm:$IMM32),
- "xxspltiw $XT, $IMM32", IIC_VecGeneral,
- 

[PATCH] D102449: [WIP][Clang][OpenMP] Add the support for compare clause in atomic directive

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

Yes, there are a couple of issues of implicit cast to be fixed. Current test is 
just a placeholder. I copied and pasted from the one for `update` clause and 
just replaced all `update`s with `compare`. I’m also working on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102449

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

[PATCH] D34654: Allow passing a regex for headers to exclude from clang-tidy

[Mateusz via Phabricator via cfe-commits]

Ashimaru added a comment.

In my company we are using a lot of standard checks and it becomes time 
consuming - this patch would be extremely useful to us and non of proposals 
montioned @alexfh seem to solve the issue.
We would like to select which directories containing headers would be checked 
and using filtering after checking still has problem of running all checks on 
file that will later discarded - and we do not like paying for something we do 
not use ;)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D34654

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

[clang] 9d7a77c - [MCParser][z/OS] Mark test as unsupported for the z/OS Target

[Fanbo Meng via cfe-commits]

Author: Fanbo Meng
Date: 2021-08-27T11:45:38-04:00
New Revision: 9d7a77c26d2f68e1c3b58b61d792d371bd3ed224

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

LOG: [MCParser][z/OS] Mark test as unsupported for the z/OS Target

Marking test as unsupported for the same reason as 
https://reviews.llvm.org/D105204

Reviewed By: abhina.sreeskantharajan

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

Added: 


Modified: 
clang/test/Driver/as-version.s

Removed: 




diff  --git a/clang/test/Driver/as-version.s b/clang/test/Driver/as-version.s
index e9e66563942f..296f9e83ece9 100644
--- a/clang/test/Driver/as-version.s
+++ b/clang/test/Driver/as-version.s
@@ -1,5 +1,6 @@
 // Test version information.
 
+// UNSUPPORTED: -zos
 // RUN: %clang -Wa,--version -c -fintegrated-as %s -o /dev/null \
 // RUN:   | FileCheck --check-prefix=IAS %s
 // IAS: clang version



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

[PATCH] D108819: [MCParser][z/OS] Mark test as unsupported for the z/OS Target

[Fanbo Meng via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9d7a77c26d2f: [MCParser][z/OS] Mark test as unsupported for 
the z/OS Target (authored by fanbo-meng).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108819

Files:
  clang/test/Driver/as-version.s


Index: clang/test/Driver/as-version.s
===
--- clang/test/Driver/as-version.s
+++ clang/test/Driver/as-version.s
@@ -1,5 +1,6 @@
 // Test version information.
 
+// UNSUPPORTED: -zos
 // RUN: %clang -Wa,--version -c -fintegrated-as %s -o /dev/null \
 // RUN:   | FileCheck --check-prefix=IAS %s
 // IAS: clang version


Index: clang/test/Driver/as-version.s
===
--- clang/test/Driver/as-version.s
+++ clang/test/Driver/as-version.s
@@ -1,5 +1,6 @@
 // Test version information.
 
+// UNSUPPORTED: -zos
 // RUN: %clang -Wa,--version -c -fintegrated-as %s -o /dev/null \
 // RUN:   | FileCheck --check-prefix=IAS %s
 // IAS: clang version
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106393: [PowerPC][AIX] Add support for varargs for complex types on AIX

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA updated this revision to Diff 369112.
ZarkoCA added a comment.

Rebase to ToT


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106393

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix32-complex-varargs.c
  clang/test/CodeGen/ppc64-varargs-complex.c
  llvm/test/CodeGen/PowerPC/aix32-complex-vararg.ll
  llvm/test/CodeGen/PowerPC/aix64-complex-vararg.ll

Index: llvm/test/CodeGen/PowerPC/aix64-complex-vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix64-complex-vararg.ll
@@ -0,0 +1,509 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mattr=-altivec \
+; RUN:  -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefix=64BIT %s
+
+@cdbl = local_unnamed_addr global { double, double } zeroinitializer, align 8
+@cdbl1 = local_unnamed_addr global { double, double } zeroinitializer, align 8
+@cdbl2 = local_unnamed_addr global { double, double } zeroinitializer, align 8
+@cdbl3 = local_unnamed_addr global { double, double } zeroinitializer, align 8
+@cflt = local_unnamed_addr global { float, float } zeroinitializer, align 4
+@cflt1 = local_unnamed_addr global { float, float } zeroinitializer, align 4
+@cflt2 = local_unnamed_addr global { float, float } zeroinitializer, align 4
+@cflt3 = local_unnamed_addr global { float, float } zeroinitializer, align 4
+@cldbl = local_unnamed_addr global { double, double } zeroinitializer, align 8
+@cldbl1 = local_unnamed_addr global { double, double } zeroinitializer, align 8
+@cldbl2 = local_unnamed_addr global { double, double } zeroinitializer, align 8
+@cldbl3 = local_unnamed_addr global { double, double } zeroinitializer, align 8
+
+define { double, double } @foo1(double %x.coerce0, double %x.coerce1, ...) local_unnamed_addr {
+; 64BIT-LABEL: foo1:
+; 64BIT:   # %bb.0: # %entry
+; 64BIT-NEXT:ld 3, L..C0(2) # %const.0
+; 64BIT-NEXT:std 5, 64(1)
+; 64BIT-NEXT:std 6, 72(1)
+; 64BIT-NEXT:lfd 4, 64(1)
+; 64BIT-NEXT:lfd 5, 72(1)
+; 64BIT-NEXT:std 7, 80(1)
+; 64BIT-NEXT:std 8, 88(1)
+; 64BIT-NEXT:lfs 0, 0(3)
+; 64BIT-NEXT:std 9, 96(1)
+; 64BIT-NEXT:addi 3, 1, 64
+; 64BIT-NEXT:std 10, 104(1)
+; 64BIT-NEXT:fadd 3, 1, 0
+; 64BIT-NEXT:fadd 0, 2, 0
+; 64BIT-NEXT:fadd 3, 3, 4
+; 64BIT-NEXT:fadd 0, 0, 5
+; 64BIT-NEXT:lfdu 4, 16(3)
+; 64BIT-NEXT:lfd 5, 88(1)
+; 64BIT-NEXT:fadd 3, 3, 1
+; 64BIT-NEXT:fadd 0, 0, 2
+; 64BIT-NEXT:fadd 3, 3, 4
+; 64BIT-NEXT:fadd 0, 0, 5
+; 64BIT-NEXT:lfd 4, 96(1)
+; 64BIT-NEXT:lfd 5, 104(1)
+; 64BIT-NEXT:fadd 3, 3, 1
+; 64BIT-NEXT:fadd 0, 0, 2
+; 64BIT-NEXT:fadd 3, 3, 4
+; 64BIT-NEXT:fadd 0, 0, 5
+; 64BIT-NEXT:lfd 4, 112(1)
+; 64BIT-NEXT:lfd 5, 120(1)
+; 64BIT-NEXT:fadd 3, 3, 1
+; 64BIT-NEXT:fadd 0, 0, 2
+; 64BIT-NEXT:fadd 3, 3, 4
+; 64BIT-NEXT:fadd 0, 0, 5
+; 64BIT-NEXT:fadd 1, 3, 1
+; 64BIT-NEXT:fadd 0, 0, 2
+; 64BIT-NEXT:lfd 2, 128(1)
+; 64BIT-NEXT:lfd 3, 136(1)
+; 64BIT-NEXT:fadd 1, 1, 2
+; 64BIT-NEXT:fadd 2, 0, 3
+; 64BIT-NEXT:blr
+entry:
+  %arg = alloca i8*, align 8
+  %0 = bitcast i8** %arg to i8*
+  call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0)
+  call void @llvm.va_start(i8* nonnull %0)
+  %add.r = fadd double %x.coerce0, 0.00e+00
+  %add.i = fadd double %x.coerce1, 0.00e+00
+  %argp.cur = load i8*, i8** %arg, align 8
+  %argp.next = getelementptr inbounds i8, i8* %argp.cur, i64 16
+  store i8* %argp.next, i8** %arg, align 8
+  %.realp = bitcast i8* %argp.cur to double*
+  %.real = load double, double* %.realp, align 8
+  %.imagp = getelementptr inbounds i8, i8* %argp.cur, i64 8
+  %1 = bitcast i8* %.imagp to double*
+  %.imag = load double, double* %1, align 8
+  %add.r4 = fadd double %add.r, %.real
+  %add.i5 = fadd double %add.i, %.imag
+  %add.r.1 = fadd double %add.r4, %x.coerce0
+  %add.i.1 = fadd double %add.i5, %x.coerce1
+  %argp.next.1 = getelementptr inbounds i8, i8* %argp.cur, i64 32
+  %.realp.1 = bitcast i8* %argp.next to double*
+  %.real.1 = load double, double* %.realp.1, align 8
+  %.imagp.1 = getelementptr inbounds i8, i8* %argp.cur, i64 24
+  %2 = bitcast i8* %.imagp.1 to double*
+  %.imag.1 = load double, double* %2, align 8
+  %add.r4.1 = fadd double %add.r.1, %.real.1
+  %add.i5.1 = fadd double %add.i.1, %.imag.1
+  %add.r.2 = fadd double %add.r4.1, %x.coerce0
+  %add.i.2 = fadd double %add.i5.1, %x.coerce1
+  %argp.next.2 = getelementptr inbounds i8, i8* %argp.cur, i64 48
+  %.realp.2 = bitcast i8* %argp.next.1 to double*
+  %.real.2 = load double, double* %.realp.2, align 8
+  %.imagp.2 = getelementptr inbounds i8, i8* %argp.cur, i64 40
+  %3 = bitcast i8* %.imagp.2 to double*
+  %.imag.2 = load double, double* %3, align 8
+  %add.r4.2 = fadd double %add.r.2, %.real.2
+  %a

[PATCH] D108826: [SLP][LTO][WIP]Allow full SLP in LTO only at link time.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
Herald added subscribers: hiraditya, inglorion.
ABataev 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/D108826

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp


Index: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
===
--- llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -172,6 +172,10 @@
 cl::desc("The maximum number of users to visit while visiting the "
  "predecessors. This prevents compilation time increase."));
 
+static cl::opt SLPLimitToRegSize(
+"slp-limit-to-reg-size", cl::init(false), cl::Hidden,
+cl::desc("Try to vectorize using only maximal vector register size."));
+
 static cl::opt
 ViewSLPTree("view-slp-tree", cl::Hidden,
 cl::desc("Display the SLP trees with Graphviz"));
@@ -7453,7 +7457,8 @@
   const unsigned MinVF = R.getMinVecRegSize() / Sz;
   unsigned VF = Chain.size();
 
-  if (!isPowerOf2_32(Sz) || !isPowerOf2_32(VF) || VF < 2 || VF < MinVF)
+  if (!isPowerOf2_32(Sz) || !isPowerOf2_32(VF) || VF < 2 || VF < MinVF ||
+  (SLPLimitToRegSize && VF < R.getMaxVecRegSize() / Sz))
 return false;
 
   LLVM_DEBUG(dbgs() << "SLP: Analyzing " << VF << " stores at offset " << Idx
@@ -7717,6 +7722,7 @@
   Type *ScalarTy = VL[0]->getType();
   if (auto *IE = dyn_cast(VL[0]))
 ScalarTy = IE->getOperand(1)->getType();
+  unsigned MaxRegSz = R.getMaxVecRegSize() / Sz;
 
   unsigned NextInst = 0, MaxInst = VL.size();
   for (unsigned VF = MaxVF; NextInst + 1 < MaxInst && VF >= MinVF; VF /= 2) {
@@ -7737,7 +7743,8 @@
   if (!isPowerOf2_32(OpsWidth))
 continue;
 
-  if ((VF > MinVF && OpsWidth <= VF / 2) || (VF == MinVF && OpsWidth < 2))
+  if ((SLPLimitToRegSize && OpsWidth < MaxRegSz) ||
+  (VF > MinVF && OpsWidth <= VF / 2) || (VF == MinVF && OpsWidth < 2))
 break;
 
   ArrayRef Ops = VL.slice(I, OpsWidth);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6441,8 +6441,13 @@
   OptSpecifier SLPVectAliasOption =
   EnableSLPVec ? options::OPT_O_Group : options::OPT_fslp_vectorize;
   if (Args.hasFlag(options::OPT_fslp_vectorize, SLPVectAliasOption,
-   options::OPT_fno_slp_vectorize, EnableSLPVec))
+   options::OPT_fno_slp_vectorize, EnableSLPVec)) {
 CmdArgs.push_back("-vectorize-slp");
+if (IsUsingLTO) {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-slp-limit-to-reg-size");
+}
+  }
 
   ParseMPreferVectorWidth(D, Args, CmdArgs);
 


Index: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
===
--- llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -172,6 +172,10 @@
 cl::desc("The maximum number of users to visit while visiting the "
  "predecessors. This prevents compilation time increase."));
 
+static cl::opt SLPLimitToRegSize(
+"slp-limit-to-reg-size", cl::init(false), cl::Hidden,
+cl::desc("Try to vectorize using only maximal vector register size."));
+
 static cl::opt
 ViewSLPTree("view-slp-tree", cl::Hidden,
 cl::desc("Display the SLP trees with Graphviz"));
@@ -7453,7 +7457,8 @@
   const unsigned MinVF = R.getMinVecRegSize() / Sz;
   unsigned VF = Chain.size();
 
-  if (!isPowerOf2_32(Sz) || !isPowerOf2_32(VF) || VF < 2 || VF < MinVF)
+  if (!isPowerOf2_32(Sz) || !isPowerOf2_32(VF) || VF < 2 || VF < MinVF ||
+  (SLPLimitToRegSize && VF < R.getMaxVecRegSize() / Sz))
 return false;
 
   LLVM_DEBUG(dbgs() << "SLP: Analyzing " << VF << " stores at offset " << Idx
@@ -7717,6 +7722,7 @@
   Type *ScalarTy = VL[0]->getType();
   if (auto *IE = dyn_cast(VL[0]))
 ScalarTy = IE->getOperand(1)->getType();
+  unsigned MaxRegSz = R.getMaxVecRegSize() / Sz;
 
   unsigned NextInst = 0, MaxInst = VL.size();
   for (unsigned VF = MaxVF; NextInst + 1 < MaxInst && VF >= MinVF; VF /= 2) {
@@ -7737,7 +7743,8 @@
   if (!isPowerOf2_32(OpsWidth))
 continue;
 
-  if ((VF > MinVF && OpsWidth <= VF / 2) || (VF == MinVF && OpsWidth < 2))
+  if ((SLPLimitToRegSize && OpsWidth < MaxRegSz) ||
+  (VF > MinVF && OpsWidth <= VF / 2) || (VF == MinVF && OpsWidth < 2))
 break;
 
   ArrayRef Ops = VL.slice(I, OpsWidth);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6441,8 +6441,13 @@
   OptSpecifier SLPVectAliasOption =
   Enable

[PATCH] D103929: [clang] P2085R0: Consistent defaulted comparisons

[Alexandru Octavian Buțiu via Phabricator via cfe-commits]

predator5047 added a comment.

ping


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

https://reviews.llvm.org/D103929

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

[PATCH] D108826: [SLP][LTO][WIP]Allow full SLP in LTO only at link time.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added subscribers: spatel, lebedev.ri.
lebedev.ri added a comment.

I think there is something really wrong with vectorzer passes in LTO pipelines.
Can you say whether the problem you are observing is in ThinLTO, Full LTO, or 
both?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108826

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

[PATCH] D108832: [Builtins] Support ext_vector_type args for __builtin_fminf.

[Florian Hahn via Phabricator via cfe-commits]

fhahn created this revision.
fhahn added reviewers: erichkeane, rjmccall, thegameg, anemet.
fhahn requested review of this revision.
Herald added a project: clang.

At the moment, the various math builtins only accept the scalar types
defined by libm.

This patch extends __builtin_fminf to also accept ext_vector_type
arguments that can be converted to an ext_vector_type with float as
element type.

This brings Clang more in line with GCC, which already supports math
builtins with ext_vector_type arguments.

If this direction makes sense, I am planning to also convert a few other
builtins that can be extended to vectors directly, like fmax.

In addition to vector types, it would be also good to support matrix
types as follow up I think.

I tried to make sure the same error messages are emitted for the scalar
variant of __builtin_fminf, but there's a difference for the (vector,
float) variant. I still need to dig into where this is coming from.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108832

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/overloaded-math-builtins.c
  clang/test/Sema/overloaded-math-builtins.c

Index: clang/test/Sema/overloaded-math-builtins.c
===
--- clang/test/Sema/overloaded-math-builtins.c
+++ clang/test/Sema/overloaded-math-builtins.c
@@ -8,7 +8,7 @@
   float r2 = __builtin_fminf(ptr, f);
   // expected-error@-1 {{passing 'int *' to parameter of incompatible type 'float'}}
   float r3 = __builtin_fminf(v, f);
-  // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to parameter of incompatible type 'float'}}
+  // expected-error@-1 {{initializing 'float' with an expression of incompatible type 'float __attribute__((ext_vector_type(4)))' (vector of 4 'float' values)}}
   float r4 = __builtin_fminf(f, v);
   // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to parameter of incompatible type 'float'}}
 
Index: clang/test/CodeGen/overloaded-math-builtins.c
===
--- /dev/null
+++ clang/test/CodeGen/overloaded-math-builtins.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin -S -o - -emit-llvm %s | FileCheck %s
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+
+void foo(float f, double d, float4 v) {
+  // CHECK:  call float @llvm.minnum.f32(float {{.*}}, float {{.*}})
+  f = __builtin_fminf(f, f);
+
+  // CHECK:  [[CONV1:%.*]] = fptrunc double {{.*}} to float
+  // CHECK:  [[CONV2:%.*]] = fptrunc double {{.*}} to float
+  // CHECK:  call float @llvm.minnum.f32(float [[CONV1]], float [[CONV2]])
+  f = __builtin_fminf(d, d);
+
+  // CHECK:   call <4 x float> @llvm.minnum.v4f32(<4 x float> {{.*}}, <4 x float> {{.*}})
+  v = __builtin_fminf(v, v);
+};
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1976,6 +1976,10 @@
 break;
   }
 
+  case Builtin::BI__builtin_fminf:
+return SemaBuiltinOverloadedMathBuiltin(TheCall, TheCallResult,
+Context.FloatTy);
+
   case Builtin::BI__builtin_matrix_transpose:
 return SemaBuiltinMatrixTranspose(TheCall, TheCallResult);
 
@@ -16525,6 +16529,55 @@
  _2, _3, _4));
 }
 
+ExprResult Sema::SemaBuiltinOverloadedMathBuiltin(CallExpr *TheCall,
+  ExprResult CallResult,
+  QualType ElementType) {
+  if (checkArgCount(*this, TheCall, 2))
+return ExprError();
+
+  // Apply default Lvalue conversions and convert the expression to size_t.
+  auto ApplyArgumentConversions = [this](Expr *E, QualType T) {
+ExprResult Res(E);
+AssignConvertType LHSConvTy = CheckSingleAssignmentConstraints(T, Res);
+if (DiagnoseAssignmentResult(LHSConvTy, E->getBeginLoc(), T, E->getType(),
+ E, AA_Passing))
+  return ExprResult(true);
+
+ExprResult Conv = DefaultLvalueConversion(E);
+assert(
+!Conv.isInvalid() &&
+"conversion should be possible according to DiagnoseAssignmentResult");
+
+return tryConvertExprToType(Conv.get(), T);
+  };
+
+  if (auto *VecType = ElementType->getAs())
+ElementType = VecType->getElementType();
+
+  QualType ArgTy = ElementType;
+  if (auto *VecType = TheCall->getArg(0)->getType()->getAs()) {
+ArgTy = Context.getExtVectorType(ElementType, VecType->getNumElements());
+  }
+
+  ExprResult A = ApplyArgumentConversions(TheCall->getArg(0), ArgTy);
+  if (A.isInvalid())
+return A;
+
+  ExprResult B = ApplyArgumentConversions(TheCall->getArg(1), ArgTy);
+  if (B.isInvalid())
+return B;
+
+  TheCall->setType(ArgTy);
+
+  auto *D = TheCall->getCalleeDecl();
+  D->addAttr(ConstAtt

[PATCH] D108832: [Builtins] Support ext_vector_type args for __builtin_fminf.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

> This brings Clang more in line with GCC, which already supports math builtins 
> with ext_vector_type arguments.

This is unfortunately not correct. When I tried this with GCC the 
`ext_vector_type` attribute was dropped and it fell back to `float`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108832

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

[PATCH] D108832: [Builtins] Support ext_vector_type args for __builtin_fminf.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I think I would want some level of examination/analysis as to whether we want 
to do this with all of the vector-types, instead of just the `ext_vector_type`.




Comment at: clang/lib/Sema/SemaChecking.cpp:16544
+ E, AA_Passing))
+  return ExprResult(true);
+

Doing this as ExprResult(true) is absurdly jarring.



Comment at: clang/lib/Sema/SemaChecking.cpp:16554
+
+  if (auto *VecType = ElementType->getAs())
+ElementType = VecType->getElementType();

I wonder if this should just deal in `VectorType`, which is the type that 
intends to emulate the GCC vector types.  My understanding is `ExtVectorType` 
is for the clang-extended vector types.



Comment at: clang/lib/Sema/SemaChecking.cpp:16575
+
+  // Update call argument to use the possibly converted matrix argument.
+  TheCall->setArg(0, A.get());

What does this comment come from?  Is this supposed to be a 'FIXME'?  I don't 
see any other matrix stuff here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108832

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

[PATCH] D108832: [Builtins] Support ext_vector_type args for __builtin_fminf.

[Florian Hahn via Phabricator via cfe-commits]

fhahn updated this revision to Diff 369135.
fhahn added a comment.

Updated to remove stray comment.

In D108832#2969512 , @erichkeane 
wrote:

> I think I would want some level of examination/analysis as to whether we want 
> to do this with all of the vector-types, instead of just the 
> `ext_vector_type`.

I think we should extend this to all vector types and matrix types. I only went 
with ext_vector_types to keep things simple initially to make sure the 
direction is acceptable first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108832

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/overloaded-math-builtins.c
  clang/test/Sema/overloaded-math-builtins.c

Index: clang/test/Sema/overloaded-math-builtins.c
===
--- clang/test/Sema/overloaded-math-builtins.c
+++ clang/test/Sema/overloaded-math-builtins.c
@@ -8,7 +8,7 @@
   float r2 = __builtin_fminf(ptr, f);
   // expected-error@-1 {{passing 'int *' to parameter of incompatible type 'float'}}
   float r3 = __builtin_fminf(v, f);
-  // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to parameter of incompatible type 'float'}}
+  // expected-error@-1 {{initializing 'float' with an expression of incompatible type 'float __attribute__((ext_vector_type(4)))' (vector of 4 'float' values)}}
   float r4 = __builtin_fminf(f, v);
   // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to parameter of incompatible type 'float'}}
 
Index: clang/test/CodeGen/overloaded-math-builtins.c
===
--- /dev/null
+++ clang/test/CodeGen/overloaded-math-builtins.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin -S -o - -emit-llvm %s | FileCheck %s
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+
+void foo(float f, double d, float4 v) {
+  // CHECK:  call float @llvm.minnum.f32(float {{.*}}, float {{.*}})
+  f = __builtin_fminf(f, f);
+
+  // CHECK:  [[CONV1:%.*]] = fptrunc double {{.*}} to float
+  // CHECK:  [[CONV2:%.*]] = fptrunc double {{.*}} to float
+  // CHECK:  call float @llvm.minnum.f32(float [[CONV1]], float [[CONV2]])
+  f = __builtin_fminf(d, d);
+
+  // CHECK:   call <4 x float> @llvm.minnum.v4f32(<4 x float> {{.*}}, <4 x float> {{.*}})
+  v = __builtin_fminf(v, v);
+};
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1976,6 +1976,10 @@
 break;
   }
 
+  case Builtin::BI__builtin_fminf:
+return SemaBuiltinOverloadedMathBuiltin(TheCall, TheCallResult,
+Context.FloatTy);
+
   case Builtin::BI__builtin_matrix_transpose:
 return SemaBuiltinMatrixTranspose(TheCall, TheCallResult);
 
@@ -16525,6 +16529,52 @@
  _2, _3, _4));
 }
 
+ExprResult Sema::SemaBuiltinOverloadedMathBuiltin(CallExpr *TheCall,
+  ExprResult CallResult,
+  QualType ElementType) {
+  if (checkArgCount(*this, TheCall, 2))
+return ExprError();
+
+  // Apply default Lvalue conversions and convert the expression to size_t.
+  auto ApplyArgumentConversions = [this](Expr *E, QualType T) {
+ExprResult Res(E);
+AssignConvertType LHSConvTy = CheckSingleAssignmentConstraints(T, Res);
+if (DiagnoseAssignmentResult(LHSConvTy, E->getBeginLoc(), T, E->getType(),
+ E, AA_Passing))
+  return ExprResult(true);
+
+ExprResult Conv = DefaultLvalueConversion(E);
+assert(
+!Conv.isInvalid() &&
+"conversion should be possible according to DiagnoseAssignmentResult");
+
+return tryConvertExprToType(Conv.get(), T);
+  };
+
+  if (auto *VecType = ElementType->getAs())
+ElementType = VecType->getElementType();
+
+  QualType ArgTy = ElementType;
+  if (auto *VecType = TheCall->getArg(0)->getType()->getAs()) {
+ArgTy = Context.getExtVectorType(ElementType, VecType->getNumElements());
+  }
+
+  ExprResult A = ApplyArgumentConversions(TheCall->getArg(0), ArgTy);
+  if (A.isInvalid())
+return A;
+
+  ExprResult B = ApplyArgumentConversions(TheCall->getArg(1), ArgTy);
+  if (B.isInvalid())
+return B;
+
+  TheCall->setType(ArgTy);
+  auto *D = TheCall->getCalleeDecl();
+  D->addAttr(ConstAttr::CreateImplicit(Context, D->getLocation()));
+  TheCall->setArg(0, A.get());
+  TheCall->setArg(1, B.get());
+  return CallResult;
+}
+
 ExprResult Sema::SemaBuiltinMatrixTranspose(CallExpr *TheCall,
 ExprResult CallResult) {
   if (checkArgCount(*this, TheCall, 1))
Index: clang/include/clang/Sema/Sema.h
===

[PATCH] D108832: [Builtins] Support ext_vector_type args for __builtin_fminf.

[Florian Hahn via Phabricator via cfe-commits]

fhahn updated this revision to Diff 369136.
fhahn marked 2 inline comments as done.
fhahn added a comment.

Use ExprError() instead of ExprResult(true).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108832

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/overloaded-math-builtins.c
  clang/test/Sema/overloaded-math-builtins.c

Index: clang/test/Sema/overloaded-math-builtins.c
===
--- clang/test/Sema/overloaded-math-builtins.c
+++ clang/test/Sema/overloaded-math-builtins.c
@@ -8,7 +8,7 @@
   float r2 = __builtin_fminf(ptr, f);
   // expected-error@-1 {{passing 'int *' to parameter of incompatible type 'float'}}
   float r3 = __builtin_fminf(v, f);
-  // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to parameter of incompatible type 'float'}}
+  // expected-error@-1 {{initializing 'float' with an expression of incompatible type 'float __attribute__((ext_vector_type(4)))' (vector of 4 'float' values)}}
   float r4 = __builtin_fminf(f, v);
   // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to parameter of incompatible type 'float'}}
 
Index: clang/test/CodeGen/overloaded-math-builtins.c
===
--- /dev/null
+++ clang/test/CodeGen/overloaded-math-builtins.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin -S -o - -emit-llvm %s | FileCheck %s
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+
+void foo(float f, double d, float4 v) {
+  // CHECK:  call float @llvm.minnum.f32(float {{.*}}, float {{.*}})
+  f = __builtin_fminf(f, f);
+
+  // CHECK:  [[CONV1:%.*]] = fptrunc double {{.*}} to float
+  // CHECK:  [[CONV2:%.*]] = fptrunc double {{.*}} to float
+  // CHECK:  call float @llvm.minnum.f32(float [[CONV1]], float [[CONV2]])
+  f = __builtin_fminf(d, d);
+
+  // CHECK:   call <4 x float> @llvm.minnum.v4f32(<4 x float> {{.*}}, <4 x float> {{.*}})
+  v = __builtin_fminf(v, v);
+};
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1976,6 +1976,10 @@
 break;
   }
 
+  case Builtin::BI__builtin_fminf:
+return SemaBuiltinOverloadedMathBuiltin(TheCall, TheCallResult,
+Context.FloatTy);
+
   case Builtin::BI__builtin_matrix_transpose:
 return SemaBuiltinMatrixTranspose(TheCall, TheCallResult);
 
@@ -16525,6 +16529,52 @@
  _2, _3, _4));
 }
 
+ExprResult Sema::SemaBuiltinOverloadedMathBuiltin(CallExpr *TheCall,
+  ExprResult CallResult,
+  QualType ElementType) {
+  if (checkArgCount(*this, TheCall, 2))
+return ExprError();
+
+  // Apply default Lvalue conversions and convert the expression to size_t.
+  auto ApplyArgumentConversions = [this](Expr *E, QualType T) {
+ExprResult Res(E);
+AssignConvertType LHSConvTy = CheckSingleAssignmentConstraints(T, Res);
+if (DiagnoseAssignmentResult(LHSConvTy, E->getBeginLoc(), T, E->getType(),
+ E, AA_Passing))
+  return ExprError();
+
+ExprResult Conv = DefaultLvalueConversion(E);
+assert(
+!Conv.isInvalid() &&
+"conversion should be possible according to DiagnoseAssignmentResult");
+
+return tryConvertExprToType(Conv.get(), T);
+  };
+
+  if (auto *VecType = ElementType->getAs())
+ElementType = VecType->getElementType();
+
+  QualType ArgTy = ElementType;
+  if (auto *VecType = TheCall->getArg(0)->getType()->getAs()) {
+ArgTy = Context.getExtVectorType(ElementType, VecType->getNumElements());
+  }
+
+  ExprResult A = ApplyArgumentConversions(TheCall->getArg(0), ArgTy);
+  if (A.isInvalid())
+return A;
+
+  ExprResult B = ApplyArgumentConversions(TheCall->getArg(1), ArgTy);
+  if (B.isInvalid())
+return B;
+
+  TheCall->setType(ArgTy);
+  auto *D = TheCall->getCalleeDecl();
+  D->addAttr(ConstAttr::CreateImplicit(Context, D->getLocation()));
+  TheCall->setArg(0, A.get());
+  TheCall->setArg(1, B.get());
+  return CallResult;
+}
+
 ExprResult Sema::SemaBuiltinMatrixTranspose(CallExpr *TheCall,
 ExprResult CallResult) {
   if (checkArgCount(*this, TheCall, 1))
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -12643,6 +12643,10 @@
 
   bool CheckPPCMMAType(QualType Type, SourceLocation TypeLoc);
 
+  ExprResult SemaBuiltinOverloadedMathBuiltin(CallExpr *TheCall,
+  ExprResult CallResult,
+  

[PATCH] D108832: [Builtins] Support ext_vector_type args for __builtin_fminf.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:16544
+ E, AA_Passing))
+  return ExprResult(true);
+

erichkeane wrote:
> Doing this as ExprResult(true) is absurdly jarring.
Updated, thanks!



Comment at: clang/lib/Sema/SemaChecking.cpp:16554
+
+  if (auto *VecType = ElementType->getAs())
+ElementType = VecType->getElementType();

erichkeane wrote:
> I wonder if this should just deal in `VectorType`, which is the type that 
> intends to emulate the GCC vector types.  My understanding is `ExtVectorType` 
> is for the clang-extended vector types.
I think we should extend this to all vector types and matrix types. I only went 
with ext_vector_types to keep things simple initially to make sure the 
direction is acceptable first.




Comment at: clang/lib/Sema/SemaChecking.cpp:16575
+
+  // Update call argument to use the possibly converted matrix argument.
+  TheCall->setArg(0, A.get());

erichkeane wrote:
> What does this comment come from?  Is this supposed to be a 'FIXME'?  I don't 
> see any other matrix stuff here.
Ah sorry, that was a left-over from where I get the boilerplate code to start 
with. It's removed in the latest version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108832

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

[PATCH] D108832: [Builtins] Support ext_vector_type args for __builtin_fminf.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Codewise I'm ok, but I don't feel comfortable making the direction decision.  
I'd like to see some level of 'state of things' on the vector types (like, 
under what conditions does GCC support this, and under what types??), but even 
then I'm not sure I'm the right one to approve this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108832

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

[clang] fe177a1 - Fix assertion when passing function into inline asm's input operand

[Jason Liu via cfe-commits]

Author: Jason Liu
Date: 2021-08-27T13:39:41-04:00
New Revision: fe177a1773e4f88dde1aa37d34a0d3f8cb582f14

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

LOG: Fix assertion when passing function into inline asm's input operand

This seem to be a regression caused by this change:
https://reviews.llvm.org/D60943.
Since we delayed report the error, we would run into some invalid
state in clang and llvm.

Without this fix, clang would assert when passing function into
inline asm's input operand.

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

Added: 
clang/test/CodeGen/asm-call-func.c

Modified: 
clang/lib/Sema/SemaStmtAsm.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaStmtAsm.cpp b/clang/lib/Sema/SemaStmtAsm.cpp
index 243d0b921cd7e..603611b2d86b1 100644
--- a/clang/lib/Sema/SemaStmtAsm.cpp
+++ b/clang/lib/Sema/SemaStmtAsm.cpp
@@ -393,30 +393,31 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, 
bool IsSimple,
   diag::err_asm_invalid_lvalue_in_input)
  << Info.getConstraintStr()
  << InputExpr->getSourceRange());
-} else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
-  if (!InputExpr->isValueDependent()) {
-Expr::EvalResult EVResult;
-if (InputExpr->EvaluateAsRValue(EVResult, Context, true)) {
-  // For compatibility with GCC, we also allow pointers that would be
-  // integral constant expressions if they were cast to int.
-  llvm::APSInt IntResult;
-  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   Context))
-if (!Info.isValidAsmImmediate(IntResult))
-  return StmtError(Diag(InputExpr->getBeginLoc(),
-diag::err_invalid_asm_value_for_constraint)
-   << toString(IntResult, 10)
-   << Info.getConstraintStr()
-   << InputExpr->getSourceRange());
-}
-  }
-
 } else {
   ExprResult Result = DefaultFunctionArrayLvalueConversion(Exprs[i]);
   if (Result.isInvalid())
 return StmtError();
 
-  Exprs[i] = Result.get();
+  InputExpr = Exprs[i] = Result.get();
+
+  if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
+if (!InputExpr->isValueDependent()) {
+  Expr::EvalResult EVResult;
+  if (InputExpr->EvaluateAsRValue(EVResult, Context, true)) {
+// For compatibility with GCC, we also allow pointers that would be
+// integral constant expressions if they were cast to int.
+llvm::APSInt IntResult;
+if (EVResult.Val.toIntegralConstant(IntResult, 
InputExpr->getType(),
+Context))
+  if (!Info.isValidAsmImmediate(IntResult))
+return StmtError(
+Diag(InputExpr->getBeginLoc(),
+ diag::err_invalid_asm_value_for_constraint)
+<< toString(IntResult, 10) << Info.getConstraintStr()
+<< InputExpr->getSourceRange());
+  }
+}
+  }
 }
 
 if (Info.allowsRegister()) {

diff  --git a/clang/test/CodeGen/asm-call-func.c 
b/clang/test/CodeGen/asm-call-func.c
new file mode 100644
index 0..851dcc093e76e
--- /dev/null
+++ b/clang/test/CodeGen/asm-call-func.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-unknown-linux-gnu| 
FileCheck %s
+
+void callee(void);
+void caller() {
+  //CHECK: call void asm sideeffect "rcall $0", 
"n,~{dirflag},~{fpsr},~{flags}"(void ()* @callee)
+  asm("rcall %0" ::"n"(callee));
+}



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

[PATCH] D107941: Fix assertion when passing function into inline asm's input operand

[Jason Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfe177a1773e4: Fix assertion when passing function into 
inline asm's input operand (authored by jasonliu).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107941

Files:
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CodeGen/asm-call-func.c


Index: clang/test/CodeGen/asm-call-func.c
===
--- /dev/null
+++ clang/test/CodeGen/asm-call-func.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-unknown-linux-gnu| 
FileCheck %s
+
+void callee(void);
+void caller() {
+  //CHECK: call void asm sideeffect "rcall $0", 
"n,~{dirflag},~{fpsr},~{flags}"(void ()* @callee)
+  asm("rcall %0" ::"n"(callee));
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -393,30 +393,31 @@
   diag::err_asm_invalid_lvalue_in_input)
  << Info.getConstraintStr()
  << InputExpr->getSourceRange());
-} else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
-  if (!InputExpr->isValueDependent()) {
-Expr::EvalResult EVResult;
-if (InputExpr->EvaluateAsRValue(EVResult, Context, true)) {
-  // For compatibility with GCC, we also allow pointers that would be
-  // integral constant expressions if they were cast to int.
-  llvm::APSInt IntResult;
-  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   Context))
-if (!Info.isValidAsmImmediate(IntResult))
-  return StmtError(Diag(InputExpr->getBeginLoc(),
-diag::err_invalid_asm_value_for_constraint)
-   << toString(IntResult, 10)
-   << Info.getConstraintStr()
-   << InputExpr->getSourceRange());
-}
-  }
-
 } else {
   ExprResult Result = DefaultFunctionArrayLvalueConversion(Exprs[i]);
   if (Result.isInvalid())
 return StmtError();
 
-  Exprs[i] = Result.get();
+  InputExpr = Exprs[i] = Result.get();
+
+  if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
+if (!InputExpr->isValueDependent()) {
+  Expr::EvalResult EVResult;
+  if (InputExpr->EvaluateAsRValue(EVResult, Context, true)) {
+// For compatibility with GCC, we also allow pointers that would be
+// integral constant expressions if they were cast to int.
+llvm::APSInt IntResult;
+if (EVResult.Val.toIntegralConstant(IntResult, 
InputExpr->getType(),
+Context))
+  if (!Info.isValidAsmImmediate(IntResult))
+return StmtError(
+Diag(InputExpr->getBeginLoc(),
+ diag::err_invalid_asm_value_for_constraint)
+<< toString(IntResult, 10) << Info.getConstraintStr()
+<< InputExpr->getSourceRange());
+  }
+}
+  }
 }
 
 if (Info.allowsRegister()) {


Index: clang/test/CodeGen/asm-call-func.c
===
--- /dev/null
+++ clang/test/CodeGen/asm-call-func.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-unknown-linux-gnu| FileCheck %s
+
+void callee(void);
+void caller() {
+  //CHECK: call void asm sideeffect "rcall $0", "n,~{dirflag},~{fpsr},~{flags}"(void ()* @callee)
+  asm("rcall %0" ::"n"(callee));
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -393,30 +393,31 @@
   diag::err_asm_invalid_lvalue_in_input)
  << Info.getConstraintStr()
  << InputExpr->getSourceRange());
-} else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
-  if (!InputExpr->isValueDependent()) {
-Expr::EvalResult EVResult;
-if (InputExpr->EvaluateAsRValue(EVResult, Context, true)) {
-  // For compatibility with GCC, we also allow pointers that would be
-  // integral constant expressions if they were cast to int.
-  llvm::APSInt IntResult;
-  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   Context))
-if (!Info.isValidAsmImmediate(IntResult))
-  return StmtError(Diag(InputExpr->getBeginLoc(),
-

[PATCH] D108826: [SLP][LTO][WIP]Allow full SLP in LTO only at link time.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D108826#2969471 , @lebedev.ri 
wrote:

> I think there is something really wrong with vectorzer passes in LTO 
> pipelines.
> Can you say whether the problem you are observing is in ThinLTO, Full LTO, or 
> both?

I saw it in Full LTO but suppose we have a similar problem in ThinLTO. SLP 
vectorizer at compile-time tries to vectorize using small vectors at it may 
affect other optimizations at link time (e.g. after inlining we may try to 
vectorize using large vector sizes etc.). This is just a preliminary attempt to 
see how can we fix this early optimization in SLP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108826

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

[PATCH] D108787: [CUDA] Pass ExecConfig through BuildCallToMemberFunction

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 369140.
tra edited the summary of this revision.
tra added a comment.

Added more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108787

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCUDA/kernel-call.cu


Index: clang/test/SemaCUDA/kernel-call.cu
===
--- clang/test/SemaCUDA/kernel-call.cu
+++ clang/test/SemaCUDA/kernel-call.cu
@@ -26,3 +26,34 @@
 
   g1<<>>(42); // expected-error {{use of undeclared identifier 
'undeclared'}}
 }
+
+// Make sure we can call static member kernels.
+template  struct a0 {
+  template  static __global__ void Call(T);
+};
+struct a1 {
+  template  static __global__ void Call(T);
+};
+template  struct a2 {
+  static __global__ void Call(T);
+};
+struct a3 {
+  static __global__ void Call(int);
+  static __global__ void Call(void*);
+};
+
+struct b {
+  template  void d0(c arg) {
+a0::Call<<<0, 0>>>(arg);
+a1::Call<<<0,0>>>(arg);
+a2::Call<<<0,0>>>(arg);
+a3::Call<<<0, 0>>>(arg);
+  }
+  void d1(void* arg) {
+a0::Call<<<0, 0>>>(arg);
+a1::Call<<<0,0>>>(arg);
+a2::Call<<<0,0>>>(arg);
+a3::Call<<<0, 0>>>(arg);
+  }
+  void e() { d0(1); }
+};
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -14166,6 +14166,7 @@
SourceLocation LParenLoc,
MultiExprArg Args,
SourceLocation RParenLoc,
+   Expr *ExecConfig, bool IsExecConfig,
bool AllowRecovery) {
   assert(MemExprE->getType() == Context.BoundMemberTy ||
  MemExprE->getType() == Context.OverloadTy);
@@ -14361,8 +14362,8 @@
 // If overload resolution picked a static member, build a
 // non-member call based on that function.
 if (Method->isStatic()) {
-  return BuildResolvedCallExpr(MemExprE, Method, LParenLoc, Args,
-   RParenLoc);
+  return BuildResolvedCallExpr(MemExprE, Method, LParenLoc, Args, 
RParenLoc,
+   ExecConfig, IsExecConfig);
 }
 
 MemExpr = cast(MemExprE->IgnoreParens());
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -6498,7 +6498,8 @@
 
 if (Fn->getType() == Context.BoundMemberTy) {
   return BuildCallToMemberFunction(Scope, Fn, LParenLoc, ArgExprs,
-   RParenLoc, AllowRecovery);
+   RParenLoc, ExecConfig, IsExecConfig,
+   AllowRecovery);
 }
   }
 
@@ -6517,7 +6518,8 @@
 Scope, Fn, ULE, LParenLoc, ArgExprs, RParenLoc, ExecConfig,
 /*AllowTypoCorrection=*/true, find.IsAddressOfOperand);
   return BuildCallToMemberFunction(Scope, Fn, LParenLoc, ArgExprs,
-   RParenLoc, AllowRecovery);
+   RParenLoc, ExecConfig, IsExecConfig,
+   AllowRecovery);
 }
   }
 
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3887,6 +3887,8 @@
SourceLocation LParenLoc,
MultiExprArg Args,
SourceLocation RParenLoc,
+   Expr *ExecConfig = nullptr,
+   bool IsExecConfig = false,
bool AllowRecovery = false);
   ExprResult
   BuildCallToObjectOfClassType(Scope *S, Expr *Object, SourceLocation 
LParenLoc,


Index: clang/test/SemaCUDA/kernel-call.cu
===
--- clang/test/SemaCUDA/kernel-call.cu
+++ clang/test/SemaCUDA/kernel-call.cu
@@ -26,3 +26,34 @@
 
   g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}}
 }
+
+// Make sure we can call static member kernels.
+template  struct a0 {
+  template  static __global__ void Call(T);
+};
+struct a1 {
+  template  static __global__ void Call(T);
+};
+template  struct a2 {
+  static __global__ void Call(T);
+};
+struct a3 {
+  static __global__ void Call(int);
+  static __global__ void Call(void*);
+};
+
+struct b {
+  template  void d0(c arg) {
+a0::Call<<<0, 0>>>(arg);
+a1::Call<<<0,0>>>(arg);
+a2::Call<<<0,0>>>(arg);
+a3::Call<<<0, 0>>>(arg);

[PATCH] D108826: [SLP][LTO][WIP]Allow full SLP in LTO only at link time.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D108826#2969547 , @ABataev wrote:

> In D108826#2969471 , @lebedev.ri 
> wrote:
>
>> I think there is something really wrong with vectorzer passes in LTO 
>> pipelines.
>> Can you say whether the problem you are observing is in ThinLTO, Full LTO, 
>> or both?
>
> I saw it in Full LTO but suppose we have a similar problem in ThinLTO. SLP 
> vectorizer at compile-time tries to vectorize using small vectors at it may 
> affect other optimizations at link time (e.g. after inlining we may try to 
> vectorize using large vector sizes etc.). This is just a preliminary attempt 
> to see how can we fix this early optimization in SLP.

Aha, so full lto. That is consistent with the phase ordering dilemma @spatel 
discovered: D102002 
IMO workarounding it in the pass isn't the right course of action. Such 
workarounds tend to stick around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108826

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

[PATCH] D108787: [CUDA] Pass ExecConfig through BuildCallToMemberFunction

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D108787#2968503 , @yaxunl wrote:

> need a test for non-template static member function as kernel. also need 
> codegen tests.

I've added more tests for different code paths leading to the kernel call. 
Interestingly enough, only the a0 actually calls `BuildCallToMemberFunction`. 
Other variants go through different code paths that handle the call without it.

As for the codegen, all kernel calls that make it to 
`clang::Sema::BuildResolvedCallExpr` with launch config are handled the same 
way. I don't think codegen tests will buy us much.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108787

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

[PATCH] D108826: [SLP][LTO][WIP]Allow full SLP in LTO only at link time.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D108826#2969594 , @lebedev.ri 
wrote:

> In D108826#2969547 , @ABataev wrote:
>
>> In D108826#2969471 , @lebedev.ri 
>> wrote:
>>
>>> I think there is something really wrong with vectorzer passes in LTO 
>>> pipelines.
>>> Can you say whether the problem you are observing is in ThinLTO, Full LTO, 
>>> or both?
>>
>> I saw it in Full LTO but suppose we have a similar problem in ThinLTO. SLP 
>> vectorizer at compile-time tries to vectorize using small vectors at it may 
>> affect other optimizations at link time (e.g. after inlining we may try to 
>> vectorize using large vector sizes etc.). This is just a preliminary attempt 
>> to see how can we fix this early optimization in SLP.
>
> Aha, so full lto. That is consistent with the phase ordering dilemma @spatel 
> discovered: D102002 

Aha, do I understand correctly that he tries to add a flag(s) that we have a 
compile without LTO, compile at LTO and link at LTO? Or something else? Or he 
just tries to reorder passes depending whether we're in LTO or not in LTO?

> IMO workarounding it in the pass isn't the right course of action. Such 
> workarounds tend to stick around.

I agree, I thought about a pipeline fix, this is just a temp solution to check 
how it affects the performance. It gets important especially for upcoming 
non-power-2 vectorization, which may cause regressions with LTO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108826

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

[clang] 2930c83 - [OpenMP][FIX] Allow declare variant to work with reference types

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2021-08-27T13:12:14-05:00
New Revision: 2930c839a5877356db6966409f4f9f2cdbcf3a07

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

LOG: [OpenMP][FIX] Allow declare variant to work with reference types

Reference types in the return or parameter position did cause the OpenMP
declare variant overload reasoning to give up. We should allow them as
we allow any other type.

This should fix the bug reported on the mailing list:
https://lists.llvm.org/pipermail/openmp-dev/2021-August/004094.html

Reviewed By: ABataev, pdhaliwal

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

Added: 
clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp

Modified: 
clang/lib/AST/ASTContext.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 591fa87d1878b..12b5d14f35c63 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -9704,11 +9704,19 @@ static QualType mergeEnumWithInteger(ASTContext 
&Context, const EnumType *ET,
 QualType ASTContext::mergeTypes(QualType LHS, QualType RHS,
 bool OfBlockPointer,
 bool Unqualified, bool BlockReturnType) {
+  // For C++ we will not reach this code with reference types (see below),
+  // for OpenMP variant call overloading we might.
+  //
   // C++ [expr]: If an expression initially has the type "reference to T", the
   // type is adjusted to "T" prior to any further analysis, the expression
   // designates the object or function denoted by the reference, and the
   // expression is an lvalue unless the reference is an rvalue reference and
   // the expression is a function call (possibly inside parentheses).
+  if (LangOpts.OpenMP && LHS->getAs() &&
+  RHS->getAs() && LHS->getTypeClass() == 
RHS->getTypeClass())
+return mergeTypes(LHS->getAs()->getPointeeType(),
+  RHS->getAs()->getPointeeType(),
+  OfBlockPointer, Unqualified, BlockReturnType);
   if (LHS->getAs() || RHS->getAs())
 return {};
 

diff  --git 
a/clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp 
b/clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
new file mode 100644
index 0..6fa8cdc59bb40
--- /dev/null
+++ b/clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
@@ -0,0 +1,414 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify -ast-dump 
%s   | FileCheck %s
+// expected-no-diagnostics
+
+// Our very own std::move, copied from libcxx.
+template  struct remove_reference { typedef _Tp type; };
+template  struct remove_reference<_Tp &> { typedef _Tp type; };
+template  struct remove_reference<_Tp &&> { typedef _Tp type; };
+
+template 
+inline typename remove_reference<_Tp>::type &&
+move(_Tp &&__t) {
+  typedef typename remove_reference<_Tp>::type _Up;
+  return static_cast<_Up &&>(__t);
+}
+// ---
+
+int Good, Bad;
+int &also_before() {
+  return Bad;
+}
+int also_before(float &&) {
+  return 0;
+}
+
+#pragma omp begin declare variant match(implementation = {vendor(score(100) \
+ : llvm)})
+int also_after(void) {
+  return 1;
+}
+int also_after(int &) {
+  return 2;
+}
+// This one does overload the int(*)(double&) version!
+int also_after(double &) {
+  return 0;
+}
+int also_after(double &&) {
+  return 3;
+}
+int also_after(short &) {
+  return 5;
+}
+int also_after(short &&) {
+  return 0;
+}
+#pragma omp end declare variant
+#pragma omp begin declare variant match(implementation = {vendor(score(0) \
+ : llvm)})
+// This one does overload the int&(*)(void) version!
+int &also_before() {
+  return Good;
+}
+// This one does *not* overload the int(*)(float&&) version!
+int also_before(float &) {
+  return 6;
+}
+#pragma omp end declare variant
+
+int also_after(void) {
+  return 7;
+}
+int also_after(int) {
+  return 8;
+}
+int also_after(double &) {
+  return 9;
+}
+int also_after(short &&) {
+  return 10;
+}
+
+int test1() {
+  // Should return 0.
+  double d;
+  return also_after(d);
+}
+
+int test2() {
+  // Should return 0.
+  return &also_before() == &Good;
+}
+
+int test3(float &&f) {
+  // Should return 0.
+  return also_before(move(f));
+}
+
+int test4(short &&s) {
+  // Should return 0.
+  return also_after(move(s));
+}
+
+int test(float &&f, short &&s) {
+  // Should return 0.
+  return test1() + test2() + test3(move(f)) + test4(move(s));
+}
+
+// CHECK:  |-ClassTemplateDecl [[ADDR_0:0x[a-z0-9]*]] <{{.*}}, col:66> 
col:29 remove_reference
+// CHECK-NEXT: | |-TemplateTypeParmDecl [[ADDR_1:0x[a-z0-9]*]]  col:17 referenced class depth 0 ind

[PATCH] D108774: [OpenMP][FIX] Allow declare variant to work with reference types

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2930c839a587: [OpenMP][FIX] Allow declare variant to work 
with reference types (authored by jdoerfert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108774

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp

Index: clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
@@ -0,0 +1,414 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify -ast-dump %s   | FileCheck %s
+// expected-no-diagnostics
+
+// Our very own std::move, copied from libcxx.
+template  struct remove_reference { typedef _Tp type; };
+template  struct remove_reference<_Tp &> { typedef _Tp type; };
+template  struct remove_reference<_Tp &&> { typedef _Tp type; };
+
+template 
+inline typename remove_reference<_Tp>::type &&
+move(_Tp &&__t) {
+  typedef typename remove_reference<_Tp>::type _Up;
+  return static_cast<_Up &&>(__t);
+}
+// ---
+
+int Good, Bad;
+int &also_before() {
+  return Bad;
+}
+int also_before(float &&) {
+  return 0;
+}
+
+#pragma omp begin declare variant match(implementation = {vendor(score(100) \
+ : llvm)})
+int also_after(void) {
+  return 1;
+}
+int also_after(int &) {
+  return 2;
+}
+// This one does overload the int(*)(double&) version!
+int also_after(double &) {
+  return 0;
+}
+int also_after(double &&) {
+  return 3;
+}
+int also_after(short &) {
+  return 5;
+}
+int also_after(short &&) {
+  return 0;
+}
+#pragma omp end declare variant
+#pragma omp begin declare variant match(implementation = {vendor(score(0) \
+ : llvm)})
+// This one does overload the int&(*)(void) version!
+int &also_before() {
+  return Good;
+}
+// This one does *not* overload the int(*)(float&&) version!
+int also_before(float &) {
+  return 6;
+}
+#pragma omp end declare variant
+
+int also_after(void) {
+  return 7;
+}
+int also_after(int) {
+  return 8;
+}
+int also_after(double &) {
+  return 9;
+}
+int also_after(short &&) {
+  return 10;
+}
+
+int test1() {
+  // Should return 0.
+  double d;
+  return also_after(d);
+}
+
+int test2() {
+  // Should return 0.
+  return &also_before() == &Good;
+}
+
+int test3(float &&f) {
+  // Should return 0.
+  return also_before(move(f));
+}
+
+int test4(short &&s) {
+  // Should return 0.
+  return also_after(move(s));
+}
+
+int test(float &&f, short &&s) {
+  // Should return 0.
+  return test1() + test2() + test3(move(f)) + test4(move(s));
+}
+
+// CHECK:  |-ClassTemplateDecl [[ADDR_0:0x[a-z0-9]*]] <{{.*}}, col:66> col:29 remove_reference
+// CHECK-NEXT: | |-TemplateTypeParmDecl [[ADDR_1:0x[a-z0-9]*]]  col:17 referenced class depth 0 index 0 _Tp
+// CHECK-NEXT: | |-CXXRecordDecl [[ADDR_2:0x[a-z0-9]*]]  col:29 struct remove_reference definition
+// CHECK-NEXT: | | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
+// CHECK-NEXT: | | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
+// CHECK-NEXT: | | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: | | | `-Destructor simple irrelevant trivial needs_implicit
+// CHECK-NEXT: | | |-CXXRecordDecl [[ADDR_3:0x[a-z0-9]*]]  col:29 implicit struct remove_reference
+// CHECK-NEXT: | | `-TypedefDecl [[ADDR_4:0x[a-z0-9]*]]  col:60 type '_Tp'
+// CHECK-NEXT: | |   `-TemplateTypeParmType [[ADDR_5:0x[a-z0-9]*]] '_Tp' dependent depth 0 index 0
+// CHECK-NEXT: | | `-TemplateTypeParm [[ADDR_1]] '_Tp'
+// CHECK-NEXT: | |-ClassTemplateSpecializationDecl [[ADDR_6:0x[a-z0-9]*]]  col:29 struct remove_reference definition
+// CHECK-NEXT: | | |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
+// CHECK-NEXT: | | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
+// CHECK-NEXT: | | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveAssignment exists simple trivial needs_implic

[clang] ed367b9 - [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

[via cfe-commits]

Author: mydeveloperday
Date: 2021-08-27T19:13:53+01:00
New Revision: ed367b9dff10ee1df9ac1984eb2ad7544da7ab06

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

LOG: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause 
C# behaviour to change

LLVM 13.0.0-rc2 shows change of behaviour in enum and interface BraceWrapping 
(likely before we simply didn't wrap)  but may be related to {D99840}

Logged as https://bugs.llvm.org/show_bug.cgi?id=51640

This change ensure AfterEnum works for

`internal|public|protected|private enum A {`  in the same way as it works for 
`enum A {` in C++

A similar issue was also observed with `interface` in C#

Reviewed By: krasimir, owenpan

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTestCSharp.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 8506eb7b88fd8..044a96e0d44e2 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3597,6 +3597,16 @@ static bool isAllmanLambdaBrace(const FormatToken &Tok) {
   !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral));
 }
 
+// Returns the first token on the line that is not a comment.
+static const FormatToken *getFirstNonComment(const AnnotatedLine &Line) {
+  const FormatToken *Next = Line.First;
+  if (!Next)
+return Next;
+  if (Next->is(tok::comment))
+Next = Next->getNextNonComment();
+  return Next;
+}
+
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
  const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
@@ -3783,12 +3793,34 @@ bool TokenAnnotator::mustBreakBefore(const 
AnnotatedLine &Line,
   if (Right.is(TT_InlineASMBrace))
 return Right.HasUnescapedNewline;
 
-  if (isAllmanBrace(Left) || isAllmanBrace(Right))
-return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
-   (Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
-Style.BraceWrapping.AfterEnum) ||
-   (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) 
||
+  if (isAllmanBrace(Left) || isAllmanBrace(Right)) {
+auto FirstNonComment = getFirstNonComment(Line);
+bool AccessSpecifier =
+FirstNonComment &&
+FirstNonComment->isOneOf(Keywords.kw_internal, tok::kw_public,
+ tok::kw_private, tok::kw_protected);
+
+if (Style.BraceWrapping.AfterEnum) {
+  if (Line.startsWith(tok::kw_enum) ||
+  Line.startsWith(tok::kw_typedef, tok::kw_enum))
+return true;
+  // Ensure BraceWrapping for `public enum A {`.
+  if (AccessSpecifier && FirstNonComment->Next &&
+  FirstNonComment->Next->is(tok::kw_enum))
+return true;
+}
+
+// Ensure BraceWrapping for `public interface A {`.
+if (Style.BraceWrapping.AfterClass &&
+((AccessSpecifier && FirstNonComment->Next &&
+  FirstNonComment->Next->is(Keywords.kw_interface)) ||
+ Line.startsWith(Keywords.kw_interface)))
+  return true;
+
+return (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) 
||
(Line.startsWith(tok::kw_struct) && 
Style.BraceWrapping.AfterStruct);
+  }
+
   if (Left.is(TT_ObjCBlockLBrace) &&
   Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
 return true;

diff  --git a/clang/unittests/Format/FormatTestCSharp.cpp 
b/clang/unittests/Format/FormatTestCSharp.cpp
index 8cd29e5548175..b7ea8d3672a2a 100644
--- a/clang/unittests/Format/FormatTestCSharp.cpp
+++ b/clang/unittests/Format/FormatTestCSharp.cpp
@@ -402,7 +402,9 @@ TEST_F(FormatTestCSharp, CSharpRegions) {
 }
 
 TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
-  verifyFormat("public enum var {\n"
+  // AfterEnum is true by default.
+  verifyFormat("public enum var\n"
+   "{\n"
"none,\n"
"@string,\n"
"bool,\n"
@@ -1099,5 +1101,218 @@ class A {
getGoogleStyle(FormatStyle::LK_Cpp));
 }
 
+TEST_F(FormatTestCSharp, CSharpAfterEnum) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterEnum = false;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("internal enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("public enum MyEnum {\n"
+   "  Foo,\n"
+   "  B

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

[MyDeveloperDay 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 rGed367b9dff10: [clang-format] [PR51640] - New AfterEnum brace 
wrapping changes have cause C#… (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108810

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -402,7 +402,9 @@
 }
 
 TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
-  verifyFormat("public enum var {\n"
+  // AfterEnum is true by default.
+  verifyFormat("public enum var\n"
+   "{\n"
"none,\n"
"@string,\n"
"bool,\n"
@@ -1099,5 +1101,218 @@
getGoogleStyle(FormatStyle::LK_Cpp));
 }
 
+TEST_F(FormatTestCSharp, CSharpAfterEnum) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterEnum = false;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("internal enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("public enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("protected enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("private enum MyEnum {\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterEnum = true;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("internal enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("public enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("protected enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("private enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("/* Foo */ private enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+  verifyFormat("/* Foo */ /* Bar */ private enum MyEnum\n"
+   "{\n"
+   "  Foo,\n"
+   "  Bar,\n"
+   "}",
+   Style);
+}
+
+TEST_F(FormatTestCSharp, CSharpAfterClass) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = false;
+
+  verifyFormat("class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("public class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("protected class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("private class MyClass {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+
+  verifyFormat("interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("internal interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("public interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("protected interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style);
+  verifyFormat("private interface Interface {\n"
+   "  int a;\n"
+   "  int b;\n"
+   "}",
+   Style)

[PATCH] D108826: [SLP][LTO][WIP]Allow full SLP in LTO only at link time.

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

In D108826#2969604 , @ABataev wrote:

> In D108826#2969594 , @lebedev.ri 
> wrote:
>
>> Aha, so full lto. That is consistent with the phase ordering dilemma @spatel 
>> discovered: D102002 
>
> Aha, do I understand correctly that he tries to add a flag(s) that we have a 
> compile without LTO, compile at LTO and link at LTO? Or something else? Or he 
> just tries to reorder passes depending whether we're in LTO or not in LTO?

We found that there were differences between regular and LTO for the passes 
invoked, their orderings, and parameters used to enable extra optimizations. 
(There was also inconsistency between new and old pass manager, but we can 
probably just focus on NPM now.)
I suspect that almost none of those differences were intentional - people just 
made changes for whatever pipeline they were interested in at the time and 
didn't realize there was divergence.
So we now have things refactored with this note:

  /// TODO: Should LTO cause any differences to this set of passes?
  void PassBuilder::addVectorPasses(OptimizationLevel Level,
FunctionPassManager &FPM, bool IsFullLTO) {

So if there really is a reason for something to be different with LTO, it's set 
up to make that easily visible at least. :)
I made a couple of small fixes in there already, but basically any place where 
we do something differently for FullLTO should be investigated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108826

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

[PATCH] D108826: [SLP][LTO][WIP]Allow full SLP in LTO only at link time.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D108826#2969677 , @spatel wrote:

> In D108826#2969604 , @ABataev wrote:
>
>> In D108826#2969594 , @lebedev.ri 
>> wrote:
>>
>>> Aha, so full lto. That is consistent with the phase ordering dilemma 
>>> @spatel discovered: D102002 
>>
>> Aha, do I understand correctly that he tries to add a flag(s) that we have a 
>> compile without LTO, compile at LTO and link at LTO? Or something else? Or 
>> he just tries to reorder passes depending whether we're in LTO or not in LTO?
>
> We found that there were differences between regular and LTO for the passes 
> invoked, their orderings, and parameters used to enable extra optimizations. 
> (There was also inconsistency between new and old pass manager, but we can 
> probably just focus on NPM now.)
> I suspect that almost none of those differences were intentional - people 
> just made changes for whatever pipeline they were interested in at the time 
> and didn't realize there was divergence.
> So we now have things refactored with this note:
>
>   /// TODO: Should LTO cause any differences to this set of passes?
>   void PassBuilder::addVectorPasses(OptimizationLevel Level,
> FunctionPassManager &FPM, bool IsFullLTO) 
> {
>
> So if there really is a reason for something to be different with LTO, it's 
> set up to make that easily visible at least. :)
> I made a couple of small fixes in there already, but basically any place 
> where we do something differently for FullLTO should be investigated.

Do I understand correctly that your patch just reorders passes? Because I need 
a bit different. We need to run SLP only for the widest possible VF at compile 
time and run SLP at full (for all possible VFs) only at link time.
Early optimization using small VF may affect other passes(alias analysis, 
loads/stores/allocas elimination, Loop Vectorization, SLP itself, etc.) and we 
need to run SLP at full only at link time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108826

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

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

[Steven Wan via Phabricator via cfe-commits]

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

Make TypeInfo carry more information about "aligned" attribute


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
  clang/test/Layout/aix-power-alignment-typedef.cpp
  clang/test/Layout/aix-type-align-and-pack-attr.cpp

Index: clang/test/Layout/aix-type-align-and-pack-attr.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-type-align-and-pack-attr.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+namespace test1 {
+struct __attribute__((__aligned__(2))) S {
+  double d;
+};
+
+S s;
+
+// CHECK: @{{.*}}test1{{.*}}s{{.*}} = global %"struct.test1::S" zeroinitializer, align 8
+} // namespace test1
+
+namespace test2 {
+struct __attribute__((__aligned__(2), packed)) S {
+  double d;
+};
+
+S s;
+
+// CHECK: @{{.*}}test2{{.*}}s{{.*}} = global %"struct.test2::S" zeroinitializer, align 2
+} // namespace test2
+
+namespace test3 {
+struct __attribute__((__aligned__(16))) S {
+  double d;
+};
+
+S s;
+
+// CHECK: @{{.*}}test3{{.*}}s{{.*}} = global %"struct.test3::S" zeroinitializer, align 16
+} // namespace test3
+
+namespace test4 {
+struct __attribute__((aligned(2))) SS {
+  double d;
+};
+
+struct S {
+  struct SS ss;
+} s;
+
+// CHECK: @{{.*}}test4{{.*}}s{{.*}} = global %"struct.test4::S" zeroinitializer, align 8
+} // namespace test4
+
+namespace test5 {
+struct __attribute__((aligned(2), packed)) SS {
+  double d;
+};
+
+struct S {
+  struct SS ss;
+} s;
+
+// CHECK: @{{.*}}test5{{.*}}s{{.*}} = global %"struct.test5::S" zeroinitializer, align 2
+} // namespace test5
Index: clang/test/Layout/aix-power-alignment-typedef.cpp
===
--- clang/test/Layout/aix-power-alignment-typedef.cpp
+++ clang/test/Layout/aix-power-alignment-typedef.cpp
@@ -37,3 +37,39 @@
 // CHECK-NEXT:   |  nvsize=2, nvalign=2, preferrednvalign=2]
 
 } // namespace test2
+
+namespace test3 {
+typedef double DblArr[] __attribute__((__aligned__(2)));
+
+union U {
+  DblArr da;
+  char x;
+};
+
+int x = sizeof(U);
+
+// CHECK:  0 | union test3::U
+// CHECK-NEXT: 0 |   test3::DblArr da
+// CHECK-NEXT: 0 |   char x
+// CHECK-NEXT:   | [sizeof=2, dsize=2, align=2, preferredalign=2,
+// CHECK-NEXT:   |  nvsize=2, nvalign=2, preferrednvalign=2]
+
+} // namespace test3
+
+namespace test4 {
+typedef double Dbl __attribute__((__aligned__(2)));
+
+union U {
+  Dbl DblArr[];
+  char x;
+};
+
+int x = sizeof(U);
+
+// CHECK:  0 | union test4::U
+// CHECK-NEXT: 0 |   test4::Dbl [] DblArr
+// CHECK-NEXT: 0 |   char x
+// CHECK-NEXT:   | [sizeof=2, dsize=2, align=2, preferredalign=2,
+// CHECK-NEXT:   |  nvsize=2, nvalign=2, preferrednvalign=2]
+
+} // namespace test4
Index: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
===
--- clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
-// RUN:   FileCheck %s
-
-// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
-// RUN:   FileCheck %s
-
-namespace test1 {
-struct __attribute__((__aligned__(2))) C {
-  double x;
-} c;
-
-// CHECK: @{{.*}}test1{{.*}}c{{.*}} = global %"struct.test1::C" zeroinitializer, align 8
-} // namespace test1
-
-namespace test2 {
-struct __attribute__((__aligned__(2), packed)) C {
-  double x;
-} c;
-
-// CHECK: @{{.*}}test2{{.*}}c{{.*}} = global %"struct.test2::C" zeroinitializer, align 2
-} // namespace test2
-
-namespace test3 {
-struct __attribute__((__aligned__(16))) C {
-  double x;
-} c;
-
-// CHECK: @{{.*}}test3{{.*}}c{{.*}} = global %"struct.test3::C" zeroinitializer, align 16
-} // namespace test3
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1831,7 +1831,7 @@
 
 // Pass over-aligned aggregates on Windows indirectly. This behavior was
 // added in MSVC 2015.
-if (IsWin32StructABI && TI.AlignIsRequired && TI.Align > 32)
+if (IsWin32StructABI && TI.isAlignRequired() && TI.Align > 32)
   return getIndirectResult(Ty, /*ByVal=*/false, State);
 
 // Expand small (<= 128-bit) record types when we know that the stack layout
@@ -6

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

[Di Mo via Phabricator via cfe-commits]

modimo updated this revision to Diff 369158.
modimo added a comment.

Check for CachedAttributes count in map rather than value so conservative 
results are not re-calculated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D36850

Files:
  clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
  clang/test/CodeGen/thinlto-distributed-cfi.ll
  clang/test/CodeGen/thinlto-funcattr-prop.ll
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/Transforms/IPO/FunctionAttrs.h
  llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/ModuleSummaryIndex.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/test/Assembler/thinlto-summary.ll
  llvm/test/Bitcode/thinlto-function-summary-refgraph.ll
  llvm/test/Bitcode/thinlto-type-vcalls.ll
  llvm/test/ThinLTO/X86/deadstrip.ll
  llvm/test/ThinLTO/X86/dot-dumper.ll
  llvm/test/ThinLTO/X86/dot-dumper2.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-exported-internal.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-indirect.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-undefined.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-weak.ll
  llvm/test/ThinLTO/X86/funcattrs-prop.ll
  llvm/test/ThinLTO/X86/funcimport_alwaysinline.ll
  llvm/test/ThinLTO/X86/function_entry_count.ll
  llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll

Index: llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
===
--- llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
+++ llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
@@ -3,15 +3,17 @@
 ; verification error.
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/linkonce_resolution_comdat.ll -o %t2.bc
-; RUN: llvm-lto -thinlto-action=run %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g -thinlto-save-temps=%t3.
+; RUN: llvm-lto -thinlto-action=run -disable-thinlto-funcattrs=0 %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g -thinlto-save-temps=%t3.
 
 ; RUN: llvm-dis %t3.0.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT1
 ; RUN: llvm-dis %t3.1.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT2
 ; Copy from first module is prevailing and converted to weak_odr, copy
 ; from second module is preempted and converted to available_externally and
 ; removed from comdat.
-; IMPORT1: define weak_odr i32 @f(i8* %0) unnamed_addr comdat($c1) {
-; IMPORT2: define available_externally i32 @f(i8* %0) unnamed_addr {
+; IMPORT1: define weak_odr i32 @f(i8* %0) unnamed_addr [[ATTR:#[0-9]+]] comdat($c1) {
+; IMPORT2: define available_externally i32 @f(i8* %0) unnamed_addr [[ATTR:#[0-9]+]] {
+
+; CHECK-DAG: attributes [[ATTR]] = { norecurse nounwind }
 
 ; RUN: llvm-nm -o - < %t1.bc.thinlto.o | FileCheck %s --check-prefix=NM1
 ; NM1: W f
Index: llvm/test/ThinLTO/X86/function_entry_count.ll
===
--- llvm/test/ThinLTO/X86/function_entry_count.ll
+++ llvm/test/ThinLTO/X86/function_entry_count.ll
@@ -2,7 +2,7 @@
 ; RUN: opt -thinlto-bc %p/Inputs/function_entry_count.ll -write-relbf-to-summary -thin-link-bitcode-file=%t2.thinlink.bc -o %t2.bc
 
 ; First perform the thin link on the normal bitcode file.
-; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t.o -save-temps -thinlto-synthesize-entry-counts \
+; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t.o -save-temps -disable-thinlto-funcattrs=0 -thinlto-synthesize-entry-counts \
 ; RUN: -r=%t1.bc,g, \
 ; RUN: -r=%t1.bc,f,px \
 ; RUN: -r=%t1.bc,h,px \
@@ -10,15 +10,16 @@
 ; RUN: -r=%t2.bc,g,px
 ; RUN: llvm-dis -o - %t.o.1.3.import.bc | FileCheck %s
 
-; RUN: llvm-lto -thinlto-action=run -thinlto-synthesize-entry-counts -exported-symbol=f \
+; RUN: llvm-lto -thinlto-action=run -disable-thinlto-funcattrs=0 -thinlto-synthesize-entry-counts -exported-symbol=f \
 ; RUN: -exported-symbol=g -exported-symbol=h -thinlto-save-temps=%t3. %t1.bc %t2.bc
 ; RUN: llvm-dis %t3.0.3.imported.bc -o - | FileCheck %s
 
-; CHECK: define void @h() !prof ![[PROF2:[0-9]+]]
-; CHECK: define void @f(i32{{.*}}) !prof ![[PROF1:[0-9]+]]
+; CHECK: define void @h() [[ATTR:#[0-9]+]] !prof ![[PROF2:[0-9]+]]
+; CHECK: define void @f(i32{{.*}}) [[ATTR:#[0-9]+]] !prof ![[PROF1:[0-9]+]]
 ; CHECK: define available_externally void @g() !prof ![[PROF2]]
 ; CHECK-DAG: ![[PROF1]] = !{!"synthetic_function_entry_count", i64 10}
 ; CHECK-DAG: ![[PROF2]] = !{!"synthetic_function_entry_count", i64 198}
+; CHECK-DAG: attributes [[ATTR]] = { norecurse nounwind }
 
 target triple = "x86_64-unknown-linux-gnu"
 target datalayout = "e-m:e-p270:32:32-p271:32: