[PATCH] D67421: [analyzer] NFC: Move IssueHash to libAnalysis.

[Valeriy Savchenko via Phabricator via cfe-commits]

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

Awesome, thanks!


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

https://reviews.llvm.org/D67421

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

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

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

xazax.hun added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:312
+  const NoteTag *getNoteTag(
+  std::function 
Cb,
+  bool IsPrunable = false) {

The callback is taken is an rvalue reference in other `getNoteTag` APIs. I 
think these overloads should be consistent.
Also, I wonder if the caller should be able to manipulate the buffer size of 
the small string (as a template parameter), but I do not have strong feelings 
about this. 

As a side note, since Clang is using C++14, maybe the lambda captures in the 
`getNoteTag` overloads above should utilize it and capture the callback by 
move. This is more of a note to ourselves independent of this patch. 

Side note 2: maybe a modernize tidy check would be useful to discover where 
rvalue references are captured by value in lambdas?



Comment at: clang/lib/StaticAnalyzer/Checkers/NullDereference.h:23
+
+/// Returns NullDereferenceBugType.
+const BugType *getNullDereferenceBugType();

I think this comment does not really add much value.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:110
+  SmartPtrChecker *Checker = Mgr.registerChecker();
+  NullDereferenceBugTypePtr = >NullDereferenceBugType;
 }

NoQ wrote:
> vrnithinkumar wrote:
> > NoQ wrote:
> > > Wait, i don't understand again. You're taking the bug type from that 
> > > checker and using it in that checker. Why did you need to make it global? 
> > > I thought your problem was about capturing the bug type from the //raw// 
> > > null dereference checker?
> > > Wait, i don't understand again. You're taking the bug type from that 
> > > checker and using it in that checker. Why did you need to make it global? 
> > > I thought your problem was about capturing the bug type from the //raw// 
> > > null dereference checker?
> > 
> > The check for bug type is in `SmartPtrModeling` but the bug type is defined 
> > in `SmartPtrChecker`
> Aha, ok, i misunderstood again. So i guess we didn't need a new header then. 
> That said, it's an interesting layering violation that we encounter in this 
> design: it used to be up to the checker to attach a visitor and so should be 
> activating a note tag, but note tags are part of modeling, not checking.
> 
> I guess that's just how it is and it's the responsibility of the checkers to 
> inform the modeling about bug types. I guess the ultimate API could look like 
> `BugReport->activateNoteTag(NoteTagTag TT)` (where `TT` is a crying smiley 
> that cries about "tag tags" T_T). But that's a story for another time.
I hope we will be able to figure out a nicer solution at some point. But I do 
not have a better alternative now, so I am ok with committing as is.

The API Artem is suggesting looks good to me (although it is out of scope for 
the GSoC). But I think that might not solve the problem of how multiple 
checkers should share the information about those tags. (Or at least I do not 
see how.)

Note that if we had both checks in the same file we might be able to work this 
problem around using `CheckerManager::getChecker`. I am not suggesting merging 
them, only wanted to point out.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:134
+
+static std::string getSmartPtrName(const MemRegion *Region) {
+  std::string SmartPtrName = "";

I wonder whether `MemRegion::printPretty` is what we want here.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:199
+} else {
+  const auto *TrackingExpr = getFirstArgExpr(Call);
+  C.addTransition(State, C.getNoteTag([ThisRegion, TrackingExpr](

I am a bit confused. As far as I understand we ALWAYS add the note tag below, 
regardless of the first argument being null.
I think I do understand that we only trigger this note tag when there is a null 
dereference but consider the code below:

```
void f(int *p) {
  std::unique_ptr u(p); // p is not always null here.
  if (!p) {
*u; // p is always null here
  }
}
```

Do we ant to output the message that the smart pointer is constructed using a 
null value? While this is technically true, there is a later event in the path 
that uncovers the fact that `p` is null.

@NoQ what do you think? I do not have any strong feelings about this, I only 
wonder how users would react to a bug report like that.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:352
+// Gets the SVal to track for a smart pointer memory region
+SVal SmartPtrModeling::getSValForRegion(const CallEvent ,
+CheckerContext ) const {

I am a bit unhappy with this function.
I feel like it does not really have a purpose. It returns the null constant for 
default constructor calls, for every other case it just returns the first 
argument. The 

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D80858#2193970 , @tra wrote:

> What is expected to happen to device statics in anonymous name spaces? It may 
> be worth adding them to the tests.
>
> LGTM otherwise.

static device var in anonymous name space will have external linkage if it is 
referenced by host code. will add a test when committing.


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

https://reviews.llvm.org/D80858

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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D79279#2195299 , @rjmccall wrote:

> Patch looks basically okay to me, although I'll second Richard's concern that 
> we shouldn't absent-mindedly start producing overloaded `memcpy`s for 
> ordinary `__builtin_memcpy`.

Yeah I think that's a leftover from the first patch. Should I drop it? At the 
same time, address spaces are currently accidentally "working", should I drop 
that in a patch before this change? Or leave as-is?




Comment at: clang/docs/LanguageExtensions.rst:2446
+order in which they occur (and in which they are observable) can only be
+guaranteed using appropriate fences around the function call. Element size must
+therefore be a lock-free size for the target architecture. It is a runtime

rjmccall wrote:
> "*The* element size must..."  But I would suggest using "access size" 
> consistently rather than "element size".
I'm being consistent with the naming for IR, which uses "element" as well. I'm 
not enamored with the naming, but wanted to point out the purposeful 
consistency to make sure you preferred "access size". Without precedent I would 
indeed prefer "access size", but have a slight preference for consistency here. 
This is extremely weakly held preference.

(I fixed "the").



Comment at: clang/docs/LanguageExtensions.rst:2454
+and might be non-uniform throughout the operation.
+
+The builtins can be used as building blocks for different facilities:

rsmith wrote:
> jfb wrote:
> > rsmith wrote:
> > > From the above description, I think the documentation is unclear what the 
> > > types `T` and `U` are used for. I think the answer is something like:
> > > 
> > > """
> > > The types `T` and `U` are required to be trivially-copyable types, and 
> > > `byte_element_size` (if specified) must be a multiple of the size of both 
> > > types. `dst` and `src` are expected to be suitably aligned for `T` and 
> > > `U` objects, respectively.
> > > """
> > > 
> > > But... we actually get the alignment information by analyzing pointer 
> > > argument rather than from the types, just like we do for `memcpy` and 
> > > `memmove`, so maybe the latter part is not right. (What did you intend 
> > > regarding alignment for the non-atomic case?) The trivial-copyability and 
> > > divisibility checks don't seem fundamentally important to the goal of the 
> > > builtin, so I wonder if we could actually just use `void` here and remove 
> > > the extra checks. (I don't really have strong views one way or the other 
> > > on this, except that we should either document what `T` and `U` are used 
> > > for or make the builtins not care about the pointee type beyond its 
> > > qualifiers.)
> > You're right. I've removed most treatment of `T` / `U`, and updated the 
> > documentation.
> > 
> > I left the trivial copy check, but `void*` is a usual escape hatch.
> > 
> > Divisibility is now only checked for `size` / `element_size`.
> Please document the trivial copy check.
Should I bubble this to the rest of the builtin in a follow-up patch? I know 
there are cases where that'll cause issues, but I worry that it would be a 
pretty noisy diagnostic (especially if we instead bubble it to C's `memcpy` 
instead).



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2639-2640
   case Builtin::BI__builtin_mempcpy: {
+QualType DestTy = getPtrArgType(CGM, E, 0);
+QualType SrcTy = getPtrArgType(CGM, E, 1);
 Address Dest = EmitPointerWithAlignment(E->getArg(0));

rsmith wrote:
> jfb wrote:
> > rsmith wrote:
> > > Looking through implicit conversions in `getPtrArgType` here will change 
> > > the code we generate for cases like:
> > > 
> > > ```
> > > void f(volatile void *p, volatile void *q) {
> > >   memcpy(p, q, 4);
> > > }
> > > ```
> > > 
> > > ... (in C, where we permit such implicit conversions) to use a volatile 
> > > memcpy intrinsic. Is that an intentional change?
> > I'm confused... what's the difference that this makes for the pre-existing 
> > builtins? My intent was to get the `QualType` unconditionally, but I can 
> > conditionalize it if needed... However this ought to make no difference:
> > ```
> > static QualType getPtrArgType(CodeGenModule , const CallExpr *E,
> >   unsigned ArgNo) {
> >   QualType ArgTy = E->getArg(ArgNo)->IgnoreImpCasts()->getType();
> >   if (ArgTy->isArrayType())
> > return CGM.getContext().getAsArrayType(ArgTy)->getElementType();
> >   if (ArgTy->isObjCObjectPointerType())
> > return ArgTy->castAs()->getPointeeType();
> >   return ArgTy->castAs()->getPointeeType();
> > }
> > ```
> > and indeed I can't see the example you provided change in IR from one to 
> > the other. The issue I'm working around is that getting it unconditionally 
> > would make ObjC code sad when `id` is passed in as I outlined above.
> The example I gave should produce a non-volatile memcpy, and used to 

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 283121.
jfb marked 2 inline comments as done.
jfb added a comment.

Update docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-overloaded-memfns.c
  clang/test/CodeGen/ubsan-builtin-checks.c
  clang/test/CodeGen/ubsan-builtin-ctz-clz.c
  clang/test/CodeGen/ubsan-builtin-mem_overloaded.c
  clang/test/CodeGenObjC/builtin-memfns.m
  clang/test/Sema/builtin-overloaded-memfns.cpp
  clang/test/SemaCXX/constexpr-string.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/test/ubsan/TestCases/Misc/builtins-ctz-clz.cpp
  compiler-rt/test/ubsan/TestCases/Misc/builtins-mem_overloaded.cpp
  compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp

Index: compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp
===
--- compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp
+++ /dev/null
@@ -1,35 +0,0 @@
-// REQUIRES: arch=x86_64
-//
-// RUN: %clangxx -fsanitize=builtin -w %s -O3 -o %t
-// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=RECOVER
-// RUN: %clangxx -fsanitize=builtin -fno-sanitize-recover=builtin -w %s -O3 -o %t.abort
-// RUN: not %run %t.abort 2>&1 | FileCheck %s --check-prefix=ABORT
-
-void check_ctz(int n) {
-  // ABORT: builtins.cpp:[[@LINE+2]]:17: runtime error: passing zero to ctz(), which is not a valid argument
-  // RECOVER: builtins.cpp:[[@LINE+1]]:17: runtime error: passing zero to ctz(), which is not a valid argument
-  __builtin_ctz(n);
-
-  // RECOVER: builtins.cpp:[[@LINE+1]]:18: runtime error: passing zero to ctz(), which is not a valid argument
-  __builtin_ctzl(n);
-
-  // RECOVER: builtins.cpp:[[@LINE+1]]:19: runtime error: passing zero to ctz(), which is not a valid argument
-  __builtin_ctzll(n);
-}
-
-void check_clz(int n) {
-  // RECOVER: builtins.cpp:[[@LINE+1]]:17: runtime error: passing zero to clz(), which is not a valid argument
-  __builtin_clz(n);
-
-  // RECOVER: builtins.cpp:[[@LINE+1]]:18: runtime error: passing zero to clz(), which is not a valid argument
-  __builtin_clzl(n);
-
-  // RECOVER: builtins.cpp:[[@LINE+1]]:19: runtime error: passing zero to clz(), which is not a valid argument
-  __builtin_clzll(n);
-}
-
-int main() {
-  check_ctz(0);
-  check_clz(0);
-  return 0;
-}
Index: compiler-rt/test/ubsan/TestCases/Misc/builtins-mem_overloaded.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Misc/builtins-mem_overloaded.cpp
@@ -0,0 +1,93 @@
+// RUN: %clangxx -fsanitize=builtin -w %s -O3 -o %t
+// RUN: %run %t 2>&1 | FileCheck %s
+
+using uintptr_t = __UINTPTR_TYPE__;
+using size_t = __SIZE_TYPE__;
+
+void check_memcpy_align(char *dst_aligned, char *dst_misaligned, const char *src_aligned, const char *src_misaligned, size_t sz) {
+  // OK.
+  __builtin_memcpy_overloaded(dst_aligned, src_aligned, sz, 2);
+  // CHECK: builtins-mem_overloaded.cpp:[[@LINE+1]]:31: runtime error: passing pointer 0x{{[0-9a-f]*}} with invalid alignment 1 into __builtin_mem*_overloaded, element size 2
+  __builtin_memcpy_overloaded(dst_misaligned, src_aligned, sz, 2);
+  // CHECK: builtins-mem_overloaded.cpp:[[@LINE+1]]:44: runtime error: passing pointer 0x{{[0-9a-f]*}} with invalid alignment 1 into __builtin_mem*_overloaded, element size 2
+  __builtin_memcpy_overloaded(dst_aligned, src_misaligned, sz, 2);
+}
+
+void check_memmove_align(char *dst_aligned, char *dst_misaligned, const char *src_aligned, const char *src_misaligned, size_t sz) {
+  // OK.
+  __builtin_memmove_overloaded(dst_aligned, src_aligned, sz, 2);
+  // CHECK: builtins-mem_overloaded.cpp:[[@LINE+1]]:32: runtime error: passing pointer 0x{{[0-9a-f]*}} with invalid alignment 1 into __builtin_mem*_overloaded, element size 2
+  __builtin_memmove_overloaded(dst_misaligned, src_aligned, sz, 2);
+  // CHECK: builtins-mem_overloaded.cpp:[[@LINE+1]]:45: runtime error: passing pointer 0x{{[0-9a-f]*}} with invalid alignment 1 into __builtin_mem*_overloaded, element size 2
+  __builtin_memmove_overloaded(dst_aligned, src_misaligned, sz, 2);
+}
+
+void check_memset_align(char *dst_aligned, char *dst_misaligned, size_t sz) {
+  // OK.
+  __builtin_memset_overloaded(dst_aligned, 0, sz, 2);
+  // CHECK: builtins-mem_overloaded.cpp:[[@LINE+1]]:31: runtime error: passing pointer 0x{{[0-9a-f]*}} with invalid alignment 1 into __builtin_mem*_overloaded, element size 2
+  __builtin_memset_overloaded(dst_misaligned, 0, sz, 2);
+}
+
+void 

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Patch looks basically okay to me, although I'll second Richard's concern that 
we shouldn't absent-mindedly start producing overloaded `memcpy`s for ordinary 
`__builtin_memcpy`.




Comment at: clang/docs/LanguageExtensions.rst:2446
+order in which they occur (and in which they are observable) can only be
+guaranteed using appropriate fences around the function call. Element size must
+therefore be a lock-free size for the target architecture. It is a runtime

"*The* element size must..."  But I would suggest using "access size" 
consistently rather than "element size".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

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

[PATCH] D85272: [clangd] Semantic highlighting for dependent template name in template argument

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
nridge requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85272

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -686,6 +686,14 @@
   void $Function[[bar]]($TemplateParameter[[T]] $Parameter[[F]]) {
 $Parameter[[F]].$DependentName[[foo]]();
   }
+)cpp",
+  // Dependent template name
+  R"cpp(
+  template  class> struct $Class[[A]] {};
+  template 
+  using $Typedef[[W]] = $Class[[A]]<
+$TemplateParameter[[T]]::template $DependentType[[Waldo]]
+  >;
 )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -296,6 +296,17 @@
 return true;
   }
 
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc L) {
+switch (L.getArgument().getKind()) {
+case TemplateArgument::Template:
+case TemplateArgument::TemplateExpansion:
+  H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
+  break;
+default:;
+}
+return RecursiveASTVisitor::TraverseTemplateArgumentLoc(L);
+  }
+
   // findExplicitReferences will walk nested-name-specifiers and
   // find anything that can be resolved to a Decl. However, non-leaf
   // components of nested-name-specifiers which are dependent names


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -686,6 +686,14 @@
   void $Function[[bar]]($TemplateParameter[[T]] $Parameter[[F]]) {
 $Parameter[[F]].$DependentName[[foo]]();
   }
+)cpp",
+  // Dependent template name
+  R"cpp(
+  template  class> struct $Class[[A]] {};
+  template 
+  using $Typedef[[W]] = $Class[[A]]<
+$TemplateParameter[[T]]::template $DependentType[[Waldo]]
+  >;
 )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -296,6 +296,17 @@
 return true;
   }
 
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc L) {
+switch (L.getArgument().getKind()) {
+case TemplateArgument::Template:
+case TemplateArgument::TemplateExpansion:
+  H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
+  break;
+default:;
+}
+return RecursiveASTVisitor::TraverseTemplateArgumentLoc(L);
+  }
+
   // findExplicitReferences will walk nested-name-specifiers and
   // find anything that can be resolved to a Decl. However, non-leaf
   // components of nested-name-specifiers which are dependent names
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82725: [PowerPC] Implement Move to VSR Mask builtins in LLVM/Clang

[Amy Kwan via Phabricator via cfe-commits]

amyk updated this revision to Diff 283113.
amyk added a comment.

Rebased patch, and removed MC tests from the original patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82725

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll

Index: llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll
===
--- llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll
+++ llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll
@@ -64,3 +64,99 @@
   %ext = tail call i32 @llvm.ppc.altivec.vextractqm(<1 x i128> %a)
   ret i32 %ext
 }
+
+declare <16 x i8> @llvm.ppc.altivec.mtvsrbm(i64)
+declare <8 x i16> @llvm.ppc.altivec.mtvsrhm(i64)
+declare <4 x i32> @llvm.ppc.altivec.mtvsrwm(i64)
+declare <2 x i64> @llvm.ppc.altivec.mtvsrdm(i64)
+declare <1 x i128> @llvm.ppc.altivec.mtvsrqm(i64)
+
+define <16 x i8>  @test_mtvsrbm(i64 %a) {
+; CHECK-LABEL: test_mtvsrbm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mtvsrbm v2, r3
+; CHECK-NEXT:blr
+entry:
+  %mv = tail call <16 x i8> @llvm.ppc.altivec.mtvsrbm(i64 %a)
+  ret <16 x i8> %mv
+}
+
+define <16 x i8>  @test_mtvsrbmi() {
+; CHECK-LABEL: test_mtvsrbmi:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mtvsrbmi v2, 1
+; CHECK-NEXT:blr
+entry:
+  %mv = tail call <16 x i8> @llvm.ppc.altivec.mtvsrbm(i64 1)
+  ret <16 x i8> %mv
+}
+
+define <16 x i8>  @test_mtvsrbmi2() {
+; CHECK-LABEL: test_mtvsrbmi2:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mtvsrbmi v2, 255
+; CHECK-NEXT:blr
+entry:
+  %mv = tail call <16 x i8> @llvm.ppc.altivec.mtvsrbm(i64 255)
+  ret <16 x i8> %mv
+}
+
+define <16 x i8>  @test_mtvsrbmi3() {
+; CHECK-LABEL: test_mtvsrbmi3:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mtvsrbmi v2, 0
+; CHECK-NEXT:blr
+entry:
+  %mv = tail call <16 x i8> @llvm.ppc.altivec.mtvsrbm(i64 256)
+  ret <16 x i8> %mv
+}
+
+define <16 x i8>  @test_mtvsrbmi4() {
+; CHECK-LABEL: test_mtvsrbmi4:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mtvsrbmi v2, 10
+; CHECK-NEXT:blr
+entry:
+  %mv = tail call <16 x i8> @llvm.ppc.altivec.mtvsrbm(i64 266)
+  ret <16 x i8> %mv
+}
+
+define <8 x i16> @test_mtvsrhm(i64 %a) {
+; CHECK-LABEL: test_mtvsrhm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mtvsrhm v2, r3
+; CHECK-NEXT:blr
+entry:
+  %mv = tail call <8 x i16> @llvm.ppc.altivec.mtvsrhm(i64 %a)
+  ret <8 x i16> %mv
+}
+
+define <4 x i32> @test_mtvsrwm(i64 %a) {
+; CHECK-LABEL: test_mtvsrwm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mtvsrwm v2, r3
+; CHECK-NEXT:blr
+entry:
+  %mv = tail call <4 x i32> @llvm.ppc.altivec.mtvsrwm(i64 %a)
+  ret <4 x i32> %mv
+}
+
+define <2 x i64> @test_mtvsrdm(i64 %a) {
+; CHECK-LABEL: test_mtvsrdm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mtvsrdm v2, r3
+; CHECK-NEXT:blr
+entry:
+  %mv = tail call <2 x i64> @llvm.ppc.altivec.mtvsrdm(i64 %a)
+  ret <2 x i64> %mv
+}
+
+define <1 x i128> @test_mtvsrqm(i64 %a) {
+; CHECK-LABEL: test_mtvsrqm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mtvsrqm v2, r3
+; CHECK-NEXT:blr
+entry:
+  %mv = tail call <1 x i128> @llvm.ppc.altivec.mtvsrqm(i64 %a)
+  ret <1 x i128> %mv
+}
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -892,22 +892,28 @@
  []>;
   def MTVSRBM : VXForm_RD5_XO5_RS5<1602, 16, (outs vrrc:$vD), (ins g8rc:$rB),
"mtvsrbm $vD, $rB", IIC_VecGeneral,
-   []>;
+   [(set v16i8:$vD,
+ (int_ppc_altivec_mtvsrbm i64:$rB))]>;
   def MTVSRHM : VXForm_RD5_XO5_RS5<1602, 17, (outs vrrc:$vD), (ins g8rc:$rB),
"mtvsrhm $vD, $rB", IIC_VecGeneral,
-   []>;
+   [(set v8i16:$vD,
+ (int_ppc_altivec_mtvsrhm i64:$rB))]>;
   def MTVSRWM : VXForm_RD5_XO5_RS5<1602, 18, (outs vrrc:$vD), (ins g8rc:$rB),
"mtvsrwm $vD, $rB", IIC_VecGeneral,
-   []>;
+   [(set v4i32:$vD,
+ (int_ppc_altivec_mtvsrwm i64:$rB))]>;
   def MTVSRDM : VXForm_RD5_XO5_RS5<1602, 19, (outs vrrc:$vD), (ins g8rc:$rB),
"mtvsrdm $vD, $rB", IIC_VecGeneral,
-   []>;
+   [(set v2i64:$vD,
+  

[PATCH] D66564: [clang-tidy] new altera struct pack align check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies added a comment.

@njames93 Thanks for the clarification! Your suggestion worked, but then I 
realized that I was working off of an improperly worded comment, which I've 
corrected. I looked through the AST Matcher reference, and didn't find anything 
that would trigger on a templated struct declaration. If you know of one I 
missed, please let me know!

If not, could you or someone else commit this patch? I do not have write 
access. My github username is ffrankies , the 
original code for the check was written by psath . If 
the lab itself (vtsynergy ) could be credited, 
that would be awesome.


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

https://reviews.llvm.org/D66564

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

[PATCH] D82726: [PowerPC] Implement Vector Count Mask Bits builtins in LLVM/Clang

[Amy Kwan via Phabricator via cfe-commits]

amyk updated this revision to Diff 283109.
amyk added a comment.

Rebased patch and removed MC tests from the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82726

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll

Index: llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll
===
--- llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll
+++ llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll
@@ -64,3 +64,48 @@
   %ext = tail call i32 @llvm.ppc.altivec.vextractqm(<1 x i128> %a)
   ret i32 %ext
 }
+
+declare i64 @llvm.ppc.altivec.vcntmbb(<16 x i8>, i32)
+declare i64 @llvm.ppc.altivec.vcntmbh(<8 x i16>, i32)
+declare i64 @llvm.ppc.altivec.vcntmbw(<4 x i32>, i32)
+declare i64 @llvm.ppc.altivec.vcntmbd(<2 x i64>, i32)
+
+define i64 @test_vcntmbb(<16 x i8> %a) {
+; CHECK-LABEL: test_vcntmbb:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vcntmbb r3, v2, 1
+; CHECK-NEXT:blr
+entry:
+  %cnt = tail call i64 @llvm.ppc.altivec.vcntmbb(<16 x i8> %a, i32 1)
+  ret i64 %cnt
+}
+
+define i64 @test_vcntmbh(<8 x i16> %a) {
+; CHECK-LABEL: test_vcntmbh:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vcntmbh r3, v2, 0
+; CHECK-NEXT:blr
+entry:
+  %cnt = tail call i64 @llvm.ppc.altivec.vcntmbh(<8 x i16> %a, i32 0)
+  ret i64 %cnt
+}
+
+define i64 @test_vcntmbw(<4 x i32> %a) {
+; CHECK-LABEL: test_vcntmbw:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vcntmbw r3, v2, 1
+; CHECK-NEXT:blr
+entry:
+  %cnt = tail call i64 @llvm.ppc.altivec.vcntmbw(<4 x i32> %a, i32 1)
+  ret i64 %cnt
+}
+
+define i64 @test_vcntmbd(<2 x i64> %a) {
+; CHECK-LABEL: test_vcntmbd:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vcntmbd r3, v2, 0
+; CHECK-NEXT:blr
+entry:
+  %cnt = tail call i64 @llvm.ppc.altivec.vcntmbd(<2 x i64> %a, i32 0)
+  ret i64 %cnt
+}
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -911,19 +911,23 @@
   def VCNTMBB : VXForm_RD5_MP_VB5<1602, 12, (outs g8rc:$rD),
   (ins vrrc:$vB, u1imm:$MP),
   "vcntmbb $rD, $vB, $MP", IIC_VecGeneral,
-  []>;
+  [(set i64:$rD, (int_ppc_altivec_vcntmbb
+v16i8:$vB, timm:$MP))]>;
   def VCNTMBH : VXForm_RD5_MP_VB5<1602, 13, (outs g8rc:$rD),
   (ins vrrc:$vB, u1imm:$MP),
   "vcntmbh $rD, $vB, $MP", IIC_VecGeneral,
-  []>;
+  [(set i64:$rD, (int_ppc_altivec_vcntmbh
+v8i16:$vB, timm:$MP))]>;
   def VCNTMBW : VXForm_RD5_MP_VB5<1602, 14, (outs g8rc:$rD),
   (ins vrrc:$vB, u1imm:$MP),
   "vcntmbw $rD, $vB, $MP", IIC_VecGeneral,
-  []>;
+  [(set i64:$rD, (int_ppc_altivec_vcntmbw
+v4i32:$vB, timm:$MP))]>;
   def VCNTMBD : VXForm_RD5_MP_VB5<1602, 15, (outs g8rc:$rD),
   (ins vrrc:$vB, u1imm:$MP),
   "vcntmbd $rD, $vB, $MP", IIC_VecGeneral,
-  []>;
+  [(set i64:$rD, (int_ppc_altivec_vcntmbd
+v2i64:$vB, timm:$MP))]>;
   def VSLDBI : VNForm_VTAB5_SD3<22, 0, (outs vrrc:$VRT),
 (ins vrrc:$VRA, vrrc:$VRB, u3imm:$SH),
 "vsldbi $VRT, $VRA, $VRB, $SH",
Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -441,6 +441,20 @@
   def int_ppc_altivec_vextractqm : GCCBuiltin<"__builtin_altivec_vextractqm">,
   Intrinsic<[llvm_i32_ty], [llvm_v1i128_ty], [IntrNoMem]>;
 
+  // P10 Vector Count with Mask intrinsics.
+  def int_ppc_altivec_vcntmbb : GCCBuiltin<"__builtin_altivec_vcntmbb">,
+  Intrinsic<[llvm_i64_ty], [llvm_v16i8_ty, llvm_i32_ty],
+[IntrNoMem, ImmArg>]>;
+  def int_ppc_altivec_vcntmbh : GCCBuiltin<"__builtin_altivec_vcntmbh">,
+  Intrinsic<[llvm_i64_ty], [llvm_v8i16_ty, llvm_i32_ty],
+[IntrNoMem, ImmArg>]>;
+  def int_ppc_altivec_vcntmbw : GCCBuiltin<"__builtin_altivec_vcntmbw">,
+  

[PATCH] D67421: [analyzer] NFC: Move IssueHash to libAnalysis.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 283108.
NoQ added a comment.
Herald added a subscriber: steakhal.

Get rid of the "V1" suffix. Indeed, we'll just rename the API.


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

https://reviews.llvm.org/D67421

Files:
  clang/include/clang/Analysis/IssueHash.h
  clang/include/clang/StaticAnalyzer/Core/IssueHash.h
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Analysis/IssueHash.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/IssueHash.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

Index: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -10,6 +10,7 @@
 //
 //===--===//
 
+#include "clang/Analysis/IssueHash.h"
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/PlistSupport.h"
@@ -20,7 +21,6 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/TokenConcatenation.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
-#include "clang/StaticAnalyzer/Core/IssueHash.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
@@ -696,7 +696,7 @@
 : D->getLocation().asLocation()),
 SM);
 const Decl *DeclWithIssue = D->getDeclWithIssue();
-EmitString(o, GetIssueHash(SM, L, D->getCheckerName(), D->getBugType(),
+EmitString(o, getIssueHash(L, D->getCheckerName(), D->getBugType(),
DeclWithIssue, LangOpts))
 << '\n';
 
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -10,6 +10,7 @@
 //
 //===--===//
 
+#include "clang/Analysis/IssueHash.h"
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
@@ -23,7 +24,6 @@
 #include "clang/Lex/Token.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "clang/StaticAnalyzer/Core/IssueHash.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
@@ -583,8 +583,8 @@
 os  << "\n\n";
 
 os << "\n\n";
 
 os << 

[PATCH] D66564: [clang-tidy] new altera struct pack align check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies updated this revision to Diff 283106.
ffrankies added a comment.

Clarified a comment: we don't want the warnings to trigger on templated struct 
//declarations//, not instantiations. We don't want it to trigger on any 
instantiations.

Added a test case that checks if warnings are triggered on instantiation of a 
badly aligned struct.


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

https://reviews.llvm.org/D66564

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
  clang-tools-extra/clang-tidy/altera/CMakeLists.txt
  clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp
  clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/altera-struct-pack-align.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/checkers/altera-struct-pack-align.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/altera-struct-pack-align.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/altera-struct-pack-align.cpp
@@ -0,0 +1,101 @@
+// RUN: %check_clang_tidy %s altera-struct-pack-align %t -- -header-filter=.*
+
+// Struct needs both alignment and packing
+struct error {
+  char a;
+  double b;
+  char c;
+};
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'error' is inefficient due to padding; only needs 10 bytes but is using 24 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: note: use "__attribute__((packed))" to reduce the amount of padding applied to struct 'error'
+// CHECK-MESSAGES: :[[@LINE-7]]:8: warning: accessing fields in struct 'error' is inefficient due to poor alignment; currently aligned to 8 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-8]]:8: note: use "__attribute__((aligned(16)))" to align struct 'error' to 16 bytes
+// CHECK-FIXES: __attribute__((packed))
+// CHECK-FIXES: __attribute__((aligned(16)));
+
+// Struct is explicitly packed, but needs alignment
+struct error_packed {
+  char a;
+  double b;
+  char c;
+} __attribute__((packed));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'error_packed' is inefficient due to poor alignment; currently aligned to 1 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: note: use "__attribute__((aligned(16)))" to align struct 'error_packed' to 16 bytes
+// CHECK-FIXES: __attribute__((aligned(16)))
+
+// Struct is properly packed, but needs alignment
+struct align_only {
+  char a;
+  char b;
+  char c;
+  char d;
+  int e;
+  double f;
+};
+// CHECK-MESSAGES: :[[@LINE-8]]:8: warning: accessing fields in struct 'align_only' is inefficient due to poor alignment; currently aligned to 8 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-9]]:8: note: use "__attribute__((aligned(16)))" to align struct 'align_only' to 16 bytes
+// CHECK-FIXES: __attribute__((aligned(16)));
+
+// Struct is perfectly packed but wrongly aligned
+struct bad_align {
+  char a;
+  double b;
+  char c;
+} __attribute__((packed)) __attribute__((aligned(8)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align' is inefficient due to poor alignment; currently aligned to 8 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: note: use "__attribute__((aligned(16)))" to align struct 'bad_align' to 16 bytes
+// CHECK-FIXES: __attribute__((aligned(16)));
+
+struct bad_align2 {
+  char a;
+  double b;
+  char c;
+} __attribute__((packed)) __attribute__((aligned(32)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align2' is inefficient due to poor alignment; currently aligned to 32 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: note: use "__attribute__((aligned(16)))" to align struct 'bad_align2' to 16 bytes
+// CHECK-FIXES: __attribute__((aligned(16)));
+
+struct bad_align3 {
+  char a;
+  double b;
+  char c;
+} __attribute__((packed)) __attribute__((aligned(4)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align3' is inefficient due to poor alignment; currently aligned to 4 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: note: use "__attribute__((aligned(16)))" to align struct 'bad_align3' to 16 bytes
+// CHECK-FIXES: __attribute__((aligned(16)));
+
+// Struct is both perfectly packed and aligned
+struct success {
+  char a;
+  double b;
+  char c;
+} __attribute__((packed)) __attribute__((aligned(16)));

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Thanks. I'd like @rjmccall to approve too, but the design of these intrinsics 
and the Sema and constant evaluation parts seem fine. (We don't strictly need 
constant evaluation to abort on library UB, so I think not catching the 
misalignment case is OK.)




Comment at: clang/docs/LanguageExtensions.rst:2447-2449
+therefore be a lock-free size for the target architecture. It is a runtime
+constraint violation to provide a memory locations which is aligned to less 
than
+the element size. It is a runtime constraint violation to provide a size which

"runtime constraint violation" is an odd phrase; in C, "constraint violation" 
means a diagnostic is required. Can we instead say that it results in undefined 
behavior?



Comment at: clang/docs/LanguageExtensions.rst:2454
+and might be non-uniform throughout the operation.
+
+The builtins can be used as building blocks for different facilities:

jfb wrote:
> rsmith wrote:
> > From the above description, I think the documentation is unclear what the 
> > types `T` and `U` are used for. I think the answer is something like:
> > 
> > """
> > The types `T` and `U` are required to be trivially-copyable types, and 
> > `byte_element_size` (if specified) must be a multiple of the size of both 
> > types. `dst` and `src` are expected to be suitably aligned for `T` and `U` 
> > objects, respectively.
> > """
> > 
> > But... we actually get the alignment information by analyzing pointer 
> > argument rather than from the types, just like we do for `memcpy` and 
> > `memmove`, so maybe the latter part is not right. (What did you intend 
> > regarding alignment for the non-atomic case?) The trivial-copyability and 
> > divisibility checks don't seem fundamentally important to the goal of the 
> > builtin, so I wonder if we could actually just use `void` here and remove 
> > the extra checks. (I don't really have strong views one way or the other on 
> > this, except that we should either document what `T` and `U` are used for 
> > or make the builtins not care about the pointee type beyond its qualifiers.)
> You're right. I've removed most treatment of `T` / `U`, and updated the 
> documentation.
> 
> I left the trivial copy check, but `void*` is a usual escape hatch.
> 
> Divisibility is now only checked for `size` / `element_size`.
Please document the trivial copy check.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2639-2640
   case Builtin::BI__builtin_mempcpy: {
+QualType DestTy = getPtrArgType(CGM, E, 0);
+QualType SrcTy = getPtrArgType(CGM, E, 1);
 Address Dest = EmitPointerWithAlignment(E->getArg(0));

jfb wrote:
> rsmith wrote:
> > Looking through implicit conversions in `getPtrArgType` here will change 
> > the code we generate for cases like:
> > 
> > ```
> > void f(volatile void *p, volatile void *q) {
> >   memcpy(p, q, 4);
> > }
> > ```
> > 
> > ... (in C, where we permit such implicit conversions) to use a volatile 
> > memcpy intrinsic. Is that an intentional change?
> I'm confused... what's the difference that this makes for the pre-existing 
> builtins? My intent was to get the `QualType` unconditionally, but I can 
> conditionalize it if needed... However this ought to make no difference:
> ```
> static QualType getPtrArgType(CodeGenModule , const CallExpr *E,
>   unsigned ArgNo) {
>   QualType ArgTy = E->getArg(ArgNo)->IgnoreImpCasts()->getType();
>   if (ArgTy->isArrayType())
> return CGM.getContext().getAsArrayType(ArgTy)->getElementType();
>   if (ArgTy->isObjCObjectPointerType())
> return ArgTy->castAs()->getPointeeType();
>   return ArgTy->castAs()->getPointeeType();
> }
> ```
> and indeed I can't see the example you provided change in IR from one to the 
> other. The issue I'm working around is that getting it unconditionally would 
> make ObjC code sad when `id` is passed in as I outlined above.
The example I gave should produce a non-volatile memcpy, and used to do so (we 
passed `false` as the fourth parameter to `CreateMemCpy`). With this patch, 
`getPtrArgType`will strip off the implicit conversion from `volatile void*` to 
`void*` in the argument type, so `isVolatile` below will be `true`, so I think 
it will now create a volatile memcpy for the same testcase. If that's not 
what's happening, then I'd like to understand why not :)

I'm not saying that's necessarily a bad change, but it is a change, and it's 
one we should make intentionally if we make it at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

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

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

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

xbolva00 added a comment.

>> But I think that and this are separate changes and I'd prefer not to combine 
>> them.

Yeah, right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85256

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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: compiler-rt/test/ubsan/TestCases/Misc/builtins-ctz-clz.cpp:1
+// REQUIRES: arch=x86_64
+//

Phab is confused I did a git rename of 
`compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp` and it thinks this is new, 
and I deleted the other.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Thanks for the detailed comments, I think I've addressed all of them! I also 
added UBSan support to check the builtin invocation. I think this patch is 
pretty much ready to go. A follow-up will need to add the support functions to 
compiler-rt (they're currently optional, as per 
https://reviews.llvm.org/D33240), and in cases where size is known we can 
inline them (as we do for memcpy and friends).




Comment at: clang/docs/LanguageExtensions.rst:2454
+and might be non-uniform throughout the operation.
+
+The builtins can be used as building blocks for different facilities:

rsmith wrote:
> From the above description, I think the documentation is unclear what the 
> types `T` and `U` are used for. I think the answer is something like:
> 
> """
> The types `T` and `U` are required to be trivially-copyable types, and 
> `byte_element_size` (if specified) must be a multiple of the size of both 
> types. `dst` and `src` are expected to be suitably aligned for `T` and `U` 
> objects, respectively.
> """
> 
> But... we actually get the alignment information by analyzing pointer 
> argument rather than from the types, just like we do for `memcpy` and 
> `memmove`, so maybe the latter part is not right. (What did you intend 
> regarding alignment for the non-atomic case?) The trivial-copyability and 
> divisibility checks don't seem fundamentally important to the goal of the 
> builtin, so I wonder if we could actually just use `void` here and remove the 
> extra checks. (I don't really have strong views one way or the other on this, 
> except that we should either document what `T` and `U` are used for or make 
> the builtins not care about the pointee type beyond its qualifiers.)
You're right. I've removed most treatment of `T` / `U`, and updated the 
documentation.

I left the trivial copy check, but `void*` is a usual escape hatch.

Divisibility is now only checked for `size` / `element_size`.



Comment at: clang/lib/AST/ExprConstant.cpp:8851
+  // about atomicity, but needs to check runtime constraints on size. We
+  // can't check the alignment runtime constraints.
+  APSInt ElSz;

rsmith wrote:
> We could use the same logic we use in `__builtin_is_aligned` here. For any 
> object whose value the constant evaluator can reason about, we should be able 
> to compute at least a minimal alignment (though the actual runtime alignment 
> might of course be greater).
I think the runtime alignment is really the only thing that matters here. I 
played with constexpr checking based on what `__builtin_is_aligned` does, and 
it's not particularly useful IMO.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2639-2640
   case Builtin::BI__builtin_mempcpy: {
+QualType DestTy = getPtrArgType(CGM, E, 0);
+QualType SrcTy = getPtrArgType(CGM, E, 1);
 Address Dest = EmitPointerWithAlignment(E->getArg(0));

rsmith wrote:
> Looking through implicit conversions in `getPtrArgType` here will change the 
> code we generate for cases like:
> 
> ```
> void f(volatile void *p, volatile void *q) {
>   memcpy(p, q, 4);
> }
> ```
> 
> ... (in C, where we permit such implicit conversions) to use a volatile 
> memcpy intrinsic. Is that an intentional change?
I'm confused... what's the difference that this makes for the pre-existing 
builtins? My intent was to get the `QualType` unconditionally, but I can 
conditionalize it if needed... However this ought to make no difference:
```
static QualType getPtrArgType(CodeGenModule , const CallExpr *E,
  unsigned ArgNo) {
  QualType ArgTy = E->getArg(ArgNo)->IgnoreImpCasts()->getType();
  if (ArgTy->isArrayType())
return CGM.getContext().getAsArrayType(ArgTy)->getElementType();
  if (ArgTy->isObjCObjectPointerType())
return ArgTy->castAs()->getPointeeType();
  return ArgTy->castAs()->getPointeeType();
}
```
and indeed I can't see the example you provided change in IR from one to the 
other. The issue I'm working around is that getting it unconditionally would 
make ObjC code sad when `id` is passed in as I outlined above.



Comment at: clang/lib/Sema/SemaChecking.cpp:5567
+  << (int)ElSz->getLimitedValue() << DstElSz << DstValTy
+  << DstOp->getSourceRange() << Arg->getSourceRange());
+

jfb wrote:
> I'm re-thinking these checks:
> ```
> if (ElSz->urem(DstElSz))
>   return ExprError(
>   Diag(TheCall->getBeginLoc(),
>PDiag(diag::err_atomic_builtin_ext_size_mismatches_el))
>   << (int)ElSz->getLimitedValue() << DstElSz << DstValTy
>   << DstOp->getSourceRange() << Arg->getSourceRange());
> ```
> I'm not sure we ought to have them anymore. We know that the types are 
> trivially copyable, it therefore doesn't really matter if you're copying with 
> operations smaller than the type itself. For example:
> ```
> struct Data {
>   

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 283078.
jfb marked 6 inline comments as done.
jfb added a comment.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

Address Richard's comments, add UBSan support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-overloaded-memfns.c
  clang/test/CodeGen/ubsan-builtin-checks.c
  clang/test/CodeGen/ubsan-builtin-ctz-clz.c
  clang/test/CodeGen/ubsan-builtin-mem_overloaded.c
  clang/test/CodeGenObjC/builtin-memfns.m
  clang/test/Sema/builtin-overloaded-memfns.cpp
  clang/test/SemaCXX/constexpr-string.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/test/ubsan/TestCases/Misc/builtins-ctz-clz.cpp
  compiler-rt/test/ubsan/TestCases/Misc/builtins-mem_overloaded.cpp
  compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp

Index: compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp
===
--- compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp
+++ /dev/null
@@ -1,35 +0,0 @@
-// REQUIRES: arch=x86_64
-//
-// RUN: %clangxx -fsanitize=builtin -w %s -O3 -o %t
-// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=RECOVER
-// RUN: %clangxx -fsanitize=builtin -fno-sanitize-recover=builtin -w %s -O3 -o %t.abort
-// RUN: not %run %t.abort 2>&1 | FileCheck %s --check-prefix=ABORT
-
-void check_ctz(int n) {
-  // ABORT: builtins.cpp:[[@LINE+2]]:17: runtime error: passing zero to ctz(), which is not a valid argument
-  // RECOVER: builtins.cpp:[[@LINE+1]]:17: runtime error: passing zero to ctz(), which is not a valid argument
-  __builtin_ctz(n);
-
-  // RECOVER: builtins.cpp:[[@LINE+1]]:18: runtime error: passing zero to ctz(), which is not a valid argument
-  __builtin_ctzl(n);
-
-  // RECOVER: builtins.cpp:[[@LINE+1]]:19: runtime error: passing zero to ctz(), which is not a valid argument
-  __builtin_ctzll(n);
-}
-
-void check_clz(int n) {
-  // RECOVER: builtins.cpp:[[@LINE+1]]:17: runtime error: passing zero to clz(), which is not a valid argument
-  __builtin_clz(n);
-
-  // RECOVER: builtins.cpp:[[@LINE+1]]:18: runtime error: passing zero to clz(), which is not a valid argument
-  __builtin_clzl(n);
-
-  // RECOVER: builtins.cpp:[[@LINE+1]]:19: runtime error: passing zero to clz(), which is not a valid argument
-  __builtin_clzll(n);
-}
-
-int main() {
-  check_ctz(0);
-  check_clz(0);
-  return 0;
-}
Index: compiler-rt/test/ubsan/TestCases/Misc/builtins-mem_overloaded.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Misc/builtins-mem_overloaded.cpp
@@ -0,0 +1,93 @@
+// RUN: %clangxx -fsanitize=builtin -w %s -O3 -o %t
+// RUN: %run %t 2>&1 | FileCheck %s
+
+using uintptr_t = __UINTPTR_TYPE__;
+using size_t = __SIZE_TYPE__;
+
+void check_memcpy_align(char *dst_aligned, char *dst_misaligned, const char *src_aligned, const char *src_misaligned, size_t sz) {
+  // OK.
+  __builtin_memcpy_overloaded(dst_aligned, src_aligned, sz, 2);
+  // CHECK: builtins-mem_overloaded.cpp:[[@LINE+1]]:31: runtime error: passing pointer 0x{{[0-9a-f]*}} with invalid alignment 1 into __builtin_mem*_overloaded, element size 2
+  __builtin_memcpy_overloaded(dst_misaligned, src_aligned, sz, 2);
+  // CHECK: builtins-mem_overloaded.cpp:[[@LINE+1]]:44: runtime error: passing pointer 0x{{[0-9a-f]*}} with invalid alignment 1 into __builtin_mem*_overloaded, element size 2
+  __builtin_memcpy_overloaded(dst_aligned, src_misaligned, sz, 2);
+}
+
+void check_memmove_align(char *dst_aligned, char *dst_misaligned, const char *src_aligned, const char *src_misaligned, size_t sz) {
+  // OK.
+  __builtin_memmove_overloaded(dst_aligned, src_aligned, sz, 2);
+  // CHECK: builtins-mem_overloaded.cpp:[[@LINE+1]]:32: runtime error: passing pointer 0x{{[0-9a-f]*}} with invalid alignment 1 into __builtin_mem*_overloaded, element size 2
+  __builtin_memmove_overloaded(dst_misaligned, src_aligned, sz, 2);
+  // CHECK: builtins-mem_overloaded.cpp:[[@LINE+1]]:45: runtime error: passing pointer 0x{{[0-9a-f]*}} with invalid alignment 1 into __builtin_mem*_overloaded, element size 2
+  __builtin_memmove_overloaded(dst_aligned, src_misaligned, sz, 2);
+}
+
+void check_memset_align(char *dst_aligned, char *dst_misaligned, size_t sz) {
+  // OK.
+  __builtin_memset_overloaded(dst_aligned, 0, sz, 2);
+  // CHECK: builtins-mem_overloaded.cpp:[[@LINE+1]]:31: runtime error: passing pointer 0x{{[0-9a-f]*}} with invalid alignment 1 into 

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D83273#2194869 , @echristo wrote:

> That said, it's a 10% compile time regression for compiling something like 
> the linux kernel or anything that's very explicit what flags they set.

Digging into the profile a little deeper:

  +9.20%  [.] getImpliedDisabledFeatures
  +1.46%  [.] llvm::X86::getImpliedFeatures
 - 3.00% llvm::StringMapImpl::LookupBucketFor
+ 1.55% llvm::StringMap 
>::try_emplace
- 1.11% llvm::StringMap::try_emplace<>
   + 1.11% clang::targets::X86TargetInfo::setFeatureEnabled
  +1.46%  [.] llvm::X86::getImpliedFeatures

so potentially a 13.23% compile time savings for the entire codebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83273

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

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D85256#2194892 , @xbolva00 wrote:

> Could this solve https://bugs.llvm.org/show_bug.cgi?id=40921 ?

Not directly; `GetExprRange` in `SemaChecking` apparently intentionally treats 
casts as producing an arbitrary value of the destination type:

  // I think we only want to look through implicit casts here; if the
  // user has an explicit widening cast, we should treat the value as
  // being of the new, wider type.

I'm not entirely convinced that what the comment describes is the best 
approach. If we revisited that choice then, with this patch, we would indeed 
diagnose the PR40921 case. But I think that and this are separate changes and 
I'd prefer not to combine them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85256

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

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 283072.
rsmith added a comment.

N/A


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85256

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/tautological-constant-compare.c

Index: clang/test/Sema/tautological-constant-compare.c
===
--- clang/test/Sema/tautological-constant-compare.c
+++ clang/test/Sema/tautological-constant-compare.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST -verify %s
@@ -545,9 +545,9 @@
   if (maybe >= e)
   return 0;
 
-  // For the time being, use the declared type of bit-fields rather than their
-  // length when determining whether a value is in-range.
-  // FIXME: Reconsider this.
+  // We only warn on out-of-range bitfields and expressions with limited range
+  // under -Wtantological-in-range-compare, not under -Wtype-limits, because
+  // the warning is not based on the type alone.
   struct A {
 int a : 3;
 unsigned b : 3;
@@ -555,13 +555,36 @@
 unsigned long d : 3;
   } a;
   if (a.a < 3) {}
-  if (a.a < 4) {}
+  if (a.a < 4) {} // #bitfield1
   if (a.b < 7) {}
-  if (a.b < 8) {}
+  if (a.b < 8) {} // #bitfield2
   if (a.c < 3) {}
-  if (a.c < 4) {}
+  if (a.c < 4) {} // #bitfield3
   if (a.d < 7) {}
-  if (a.d < 8) {}
+  if (a.d < 8) {} // #bitfield4
+#if TEST == 2
+  // expected-warning@#bitfield1 {{comparison of 3-bit signed value < 4 is always true}}
+  // expected-warning@#bitfield2 {{comparison of 3-bit unsigned value < 8 is always true}}
+  // expected-warning@#bitfield3 {{comparison of 3-bit signed value < 4 is always true}}
+  // expected-warning@#bitfield4 {{comparison of 3-bit unsigned value < 8 is always true}}
+#endif
+
+  if ((s & 0xff) < 0) {} // #valuerange1
+  if ((s & 0xff) < 1) {}
+  if ((s & -3) < -4) {} // #valuerange2
+  if ((s & -3) < -3) {}
+  if ((s & -3) < 4u) {} // (true if s non-negative)
+  if ((s & -3) > 4u) {} // (true if s negative)
+  if ((s & -3) == 4u) {} // #valuerange3 (never true)
+  if ((s & -3) == 3u) {}
+  if ((s & -3) == -5u) {} // #valuerange4
+  if ((s & -3) == -4u) {}
+#if TEST == 2
+  // expected-warning@#valuerange1 {{comparison of 8-bit unsigned value < 0 is always false}}
+  // expected-warning@#valuerange2 {{comparison of 3-bit signed value < -4 is always false}}
+  // expected-warning@#valuerange3 {{comparison of 3-bit signed value == 4 is always false}}
+  // expected-warning@#valuerange4 {{comparison of 3-bit signed value == 4294967291 is always false}}
+#endif
 
   return 1;
 }
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10792,15 +10792,16 @@
   S.Context.hasSameUnqualifiedType(Constant->getType(), Other->getType()))
 return false;
 
-  // TODO: Investigate using GetExprRange() to get tighter bounds
-  // on the bit ranges.
+  IntRange OtherValueRange =
+  GetExprRange(S.Context, Other, S.isConstantEvaluated());
+
   QualType OtherT = Other->getType();
   if (const auto *AT = OtherT->getAs())
 OtherT = AT->getValueType();
-  IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
+  IntRange OtherTypeRange = IntRange::forValueOfType(S.Context, OtherT);
 
   // Special case for ObjC BOOL on targets where its a typedef for a signed char
-  // (Namely, macOS).
+  // (Namely, macOS). FIXME: IntRange::forValueOfType should do this.
   bool IsObjCSignedCharBool = S.getLangOpts().ObjC &&
   S.NSAPIObj->isObjCBOOLType(OtherT) &&
   OtherT->isSpecificBuiltinType(BuiltinType::SChar);
@@ -10810,17 +10811,32 @@
   bool OtherIsBooleanDespiteType =
   !OtherT->isBooleanType() && Other->isKnownToHaveBooleanValue();
   if (OtherIsBooleanDespiteType || IsObjCSignedCharBool)
-OtherRange = IntRange::forBoolType();
+OtherTypeRange = OtherValueRange = IntRange::forBoolType();
 
-  // Determine the promoted range of the other type and see if a 

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 283070.
rsmith added a comment.

Unbreak test that was autobroken by lint.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85256

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/tautological-constant-compare.c

Index: clang/test/Sema/tautological-constant-compare.c
===
--- clang/test/Sema/tautological-constant-compare.c
+++ clang/test/Sema/tautological-constant-compare.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST -verify %s
@@ -545,9 +545,9 @@
   if (maybe >= e)
   return 0;
 
-  // For the time being, use the declared type of bit-fields rather than their
-  // length when determining whether a value is in-range.
-  // FIXME: Reconsider this.
+  // We only warn on out-of-range bitfields and expressions with limited range
+  // under -Wtantological-in-range-compare, not under -Wtype-limits, because
+  // the warning is not based on the type alone.
   struct A {
 int a : 3;
 unsigned b : 3;
@@ -555,13 +555,36 @@
 unsigned long d : 3;
   } a;
   if (a.a < 3) {}
-  if (a.a < 4) {}
+  if (a.a < 4) {} // #bitfield1
   if (a.b < 7) {}
-  if (a.b < 8) {}
+  if (a.b < 8) {} // #bitfield2
   if (a.c < 3) {}
-  if (a.c < 4) {}
+  if (a.c < 4) {} // #bitfield3
   if (a.d < 7) {}
-  if (a.d < 8) {}
+  if (a.d < 8) {} // #bitfield4
+#if TEST == 2
+  // expected-warning@#bitfield1 {{comparison of 3-bit signed value < 4 is always true}}
+  // expected-warning@#bitfield2 {{comparison of 3-bit unsigned value < 8 is always true}}
+  // expected-warning@#bitfield3 {{comparison of 3-bit signed value < 4 is always true}}
+  // expected-warning@#bitfield4 {{comparison of 3-bit unsigned value < 8 is always true}}
+#endif
+
+  if ((s & 0xff) < 0) {} // #valuerange1
+  if ((s & 0xff) < 1) {}
+  if ((s & -3) < -4) {} // #valuerange2
+  if ((s & -3) < -3) {}
+  if ((s & -3) < 4u) {} // (true if s non-negative)
+  if ((s & -3) > 4u) {} // (true if s negative)
+  if ((s & -3) == 4u) {} // #valuerange3 (never true)
+  if ((s & -3) == 3u) {}
+  if ((s & -3) == -5u) {} // #valuerange4
+  if ((s & -3) == -4u) {}
+#if TEST == 2
+  // expected-warning@#valuerange1 {{comparison of 8-bit unsigned value < 0 is always false}}
+  // expected-warning@#valuerange2 {{comparison of 3-bit signed value < -4 is always false}}
+  // expected-warning@#valuerange3 {{comparison of 3-bit signed value == 4 is always false}}
+  // expected-warning@#valuerange4 {{comparison of 3-bit signed value == 4294967291 is always false}}
+#endif
 
   return 1;
 }
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10792,15 +10792,16 @@
   S.Context.hasSameUnqualifiedType(Constant->getType(), Other->getType()))
 return false;
 
-  // TODO: Investigate using GetExprRange() to get tighter bounds
-  // on the bit ranges.
+  IntRange OtherValueRange =
+  GetExprRange(S.Context, Other, S.isConstantEvaluated());
+
   QualType OtherT = Other->getType();
   if (const auto *AT = OtherT->getAs())
 OtherT = AT->getValueType();
-  IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
+  IntRange OtherTypeRange = IntRange::forValueOfType(S.Context, OtherT);
 
   // Special case for ObjC BOOL on targets where its a typedef for a signed char
-  // (Namely, macOS).
+  // (Namely, macOS). FIXME: IntRange::forValueOfType should do this.
   bool IsObjCSignedCharBool = S.getLangOpts().ObjC &&
   S.NSAPIObj->isObjCBOOLType(OtherT) &&
   OtherT->isSpecificBuiltinType(BuiltinType::SChar);
@@ -10810,17 +10811,32 @@
   bool OtherIsBooleanDespiteType =
   !OtherT->isBooleanType() && Other->isKnownToHaveBooleanValue();
   if (OtherIsBooleanDespiteType || IsObjCSignedCharBool)
-OtherRange = IntRange::forBoolType();
+OtherTypeRange = OtherValueRange = IntRange::forBoolType();
 
-  // Determine the promoted range 

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D71726#2182667 , @tra wrote:

> LGTM, modulo couple of nits.
>
> @jyknight are you OK with this?
>
> In D71726#2179428 , @yaxunl wrote:
>
>> Make IEEE single and double type as supported for fp atomics in all targets 
>> by default. This is based on the assumption that AtomicExpandPass or its 
>> ongoing work is sufficient to support fp atomics for all targets. This is to 
>> facilitate middle end and backend end development to support fp atomics.
>>
>> If a target would like to treat single and double fp atomics as unsupported, 
>> it can override the default behavior in its own TargetInfo.
>
> Do we have sufficient test coverage on all platforms to make sure we're not 
> generating something that LLVM can't handle everywhere?
> If not, perhaps we should default to unsupported and only enable it for known 
> working targets.

I updated TargetInfo for fp atomic support for common targets. Basically by 
default fp atomic support is now off. It is enabled only for targets which do 
not generate lib calls for fp atomics. This is because the availability of lib 
call depends on platform, so it is up to the Target owners to determine whether 
the support is available if lib call is needed. For those targets which are 
able to generate llvm fp atomic instructions, fp atomic support is enabled in 
clang, and tests are added to cover them.




Comment at: clang/lib/CodeGen/CGAtomic.cpp:889-891
+if (MemTy->isFloatingType()) {
+  ShouldCastToIntPtrTy = false;
+}

tra wrote:
> `ShouldCastToIntPtrTy = !MemTy->isFloatingType();`
done



Comment at: clang/test/Sema/atomic-ops.c:102-103
 void f(_Atomic(int) *i, const _Atomic(int) *ci,
-   _Atomic(int*) *p, _Atomic(float) *d,
+   _Atomic(int*) *p, _Atomic(float) *d, _Atomic(double) *d2,
+   _Atomic(long double) *d3,
int *I, const int *CI,

tra wrote:
> Rename arguments? d -> f, d2 -> d, d3 -> ld ?
done


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

https://reviews.llvm.org/D71726

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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 283067.
yaxunl added a comment.
Herald added subscribers: atanasyan, sdardis.

added tests for targets supporting fp atomics.


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

https://reviews.llvm.org/D71726

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/fp-atomic-ops.c
  clang/test/CodeGenCUDA/amdgpu-atomic-ops.cu
  clang/test/CodeGenOpenCL/atomic-ops.cl
  clang/test/Sema/atomic-ops.c
  clang/test/SemaCUDA/amdgpu-atomic-ops.cu
  clang/test/SemaOpenCL/atomic-ops.cl

Index: clang/test/SemaOpenCL/atomic-ops.cl
===
--- clang/test/SemaOpenCL/atomic-ops.cl
+++ clang/test/SemaOpenCL/atomic-ops.cl
@@ -1,10 +1,13 @@
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only -triple=spir64
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only -triple=amdgcn-amdhsa-amd-opencl
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify=expected,spir \
+// RUN:   -fsyntax-only -triple=spir64
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only \
+// RUN:   -triple=amdgcn-amd-amdhsa
 
 // Basic parsing/Sema tests for __opencl_atomic_*
 
 #pragma OPENCL EXTENSION cl_khr_int64_base_atomics : enable
 #pragma OPENCL EXTENSION cl_khr_int64_extended_atomics : enable
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
 
 typedef __INTPTR_TYPE__ intptr_t;
 typedef int int8 __attribute__((ext_vector_type(8)));
@@ -36,7 +39,7 @@
 
 atomic_int gn;
 void f(atomic_int *i, const atomic_int *ci,
-   atomic_intptr_t *p, atomic_float *d,
+   atomic_intptr_t *p, atomic_float *f, atomic_double *d, atomic_half *h, // expected-error {{unknown type name 'atomic_half'}}
int *I, const int *CI,
intptr_t *P, float *D, struct S *s1, struct S *s2,
global atomic_int *i_g, local atomic_int *i_l, private atomic_int *i_p,
@@ -57,37 +60,38 @@
 
   __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_load(p, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_load(d, memory_order_seq_cst, memory_scope_work_group);
+  __opencl_atomic_load(f, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_load(ci, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_load(i_c, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to non-constant _Atomic type ('__constant atomic_int *' (aka '__constant _Atomic(int) *') invalid)}}
 
   __opencl_atomic_store(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_store(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  (int)__opencl_atomic_store(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{operand of type 'void' where arithmetic or pointer type is required}}
+  (int)__opencl_atomic_store(f, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{operand of type 'void' where arithmetic or pointer type is required}}
 
   int exchange_1 = __opencl_atomic_exchange(i, 1, memory_order_seq_cst, memory_scope_work_group);
   int exchange_2 = __opencl_atomic_exchange(I, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to _Atomic}}
 
   __opencl_atomic_fetch_add(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_add(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_add(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_add(f, 1.0f, memory_order_seq_cst, memory_scope_work_group);
+  __opencl_atomic_fetch_add(d, 1.0, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_and(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_and(f, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
 
   __opencl_atomic_fetch_min(i, 1, memory_order_seq_cst, memory_scope_work_group);
   

[PATCH] D82727: [PowerPC] Implement Vector Expand Mask builtins in LLVM/Clang

[Amy Kwan via Phabricator via cfe-commits]

amyk added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:881
+ [(set v16i8:$vD, 
(int_ppc_altivec_vexpandbm
+  v16i8:$vB))]>;
   def VEXPANDHM : VXForm_RD5_XO5_RS5<1602, 1, (outs vrrc:$vD), (ins vrrc:$vB),

I have no idea why this indentation is off, since it did not appear like that 
previously. In any case, I can address this during the commit if it is OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82727

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

[PATCH] D82727: [PowerPC] Implement Vector Expand Mask builtins in LLVM/Clang

[Amy Kwan via Phabricator via cfe-commits]

amyk updated this revision to Diff 283065.
amyk added a comment.

Rebased the patch and removed MC tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82727

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll

Index: llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll
===
--- llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll
+++ llvm/test/CodeGen/PowerPC/p10-vector-mask-ops.ll
@@ -64,3 +64,59 @@
   %ext = tail call i32 @llvm.ppc.altivec.vextractqm(<1 x i128> %a)
   ret i32 %ext
 }
+
+declare <16 x i8> @llvm.ppc.altivec.vexpandbm(<16 x i8>)
+declare <8 x i16> @llvm.ppc.altivec.vexpandhm(<8 x i16>)
+declare <4 x i32> @llvm.ppc.altivec.vexpandwm(<4 x i32>)
+declare <2 x i64> @llvm.ppc.altivec.vexpanddm(<2 x i64>)
+declare <1 x i128> @llvm.ppc.altivec.vexpandqm(<1 x i128>)
+
+define <16 x i8> @test_vexpandbm(<16 x i8> %a) {
+; CHECK-LABEL: test_vexpandbm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vexpandbm v2, v2
+; CHECK-NEXT:blr
+entry:
+  %exp = tail call <16 x i8> @llvm.ppc.altivec.vexpandbm(<16 x i8> %a)
+  ret <16 x i8> %exp
+}
+
+define <8 x i16> @test_vexpandhm(<8 x i16> %a) {
+; CHECK-LABEL: test_vexpandhm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vexpandhm v2, v2
+; CHECK-NEXT:blr
+entry:
+  %exp = tail call <8 x i16> @llvm.ppc.altivec.vexpandhm(<8 x i16> %a)
+  ret <8 x i16> %exp
+}
+
+define <4 x i32> @test_vexpandwm(<4 x i32> %a) {
+; CHECK-LABEL: test_vexpandwm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vexpandwm v2, v2
+; CHECK-NEXT:blr
+entry:
+  %exp = tail call <4 x i32> @llvm.ppc.altivec.vexpandwm(<4 x i32> %a)
+  ret <4 x i32> %exp
+}
+
+define <2 x i64> @test_vexpanddm(<2 x i64> %a) {
+; CHECK-LABEL: test_vexpanddm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vexpanddm v2, v2
+; CHECK-NEXT:blr
+entry:
+  %exp = tail call <2 x i64> @llvm.ppc.altivec.vexpanddm(<2 x i64> %a)
+  ret <2 x i64> %exp
+}
+
+define <1 x i128> @test_vexpandqm(<1 x i128> %a) {
+; CHECK-LABEL: test_vexpandqm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vexpandqm v2, v2
+; CHECK-NEXT:blr
+entry:
+  %exp = tail call <1 x i128> @llvm.ppc.altivec.vexpandqm(<1 x i128> %a)
+  ret <1 x i128> %exp
+}
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -877,19 +877,24 @@
   (int_ppc_altivec_vextractqm v1i128:$vB))]>;
   def VEXPANDBM : VXForm_RD5_XO5_RS5<1602, 0, (outs vrrc:$vD), (ins vrrc:$vB),
  "vexpandbm $vD, $vB", IIC_VecGeneral,
- []>;
+ [(set v16i8:$vD, (int_ppc_altivec_vexpandbm
+	   v16i8:$vB))]>;
   def VEXPANDHM : VXForm_RD5_XO5_RS5<1602, 1, (outs vrrc:$vD), (ins vrrc:$vB),
  "vexpandhm $vD, $vB", IIC_VecGeneral,
- []>;
+ [(set v8i16:$vD, (int_ppc_altivec_vexpandhm
+   v8i16:$vB))]>;
   def VEXPANDWM : VXForm_RD5_XO5_RS5<1602, 2, (outs vrrc:$vD), (ins vrrc:$vB),
  "vexpandwm $vD, $vB", IIC_VecGeneral,
- []>;
+ [(set v4i32:$vD, (int_ppc_altivec_vexpandwm
+   v4i32:$vB))]>;
   def VEXPANDDM : VXForm_RD5_XO5_RS5<1602, 3, (outs vrrc:$vD), (ins vrrc:$vB),
  "vexpanddm $vD, $vB", IIC_VecGeneral,
- []>;
+ [(set v2i64:$vD, (int_ppc_altivec_vexpanddm
+   v2i64:$vB))]>;
   def VEXPANDQM : VXForm_RD5_XO5_RS5<1602, 4, (outs vrrc:$vD), (ins vrrc:$vB),
  "vexpandqm $vD, $vB", IIC_VecGeneral,
- []>;
+ [(set v1i128:$vD, (int_ppc_altivec_vexpandqm
+   v1i128:$vB))]>;
   def MTVSRBM : VXForm_RD5_XO5_RS5<1602, 16, (outs vrrc:$vD), (ins g8rc:$rB),
"mtvsrbm $vD, $rB", IIC_VecGeneral,
[]>;
Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -441,6 +441,18 @@
   def int_ppc_altivec_vextractqm : 

[PATCH] D85176: [Coverage] Enable emitting gap area between macros

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: clang/test/CoverageMapping/macro-expressions.cpp:63
   // CHECK-NEXT: File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:18 = #2
   if (EXPR(i)) {}
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:9 -> [[@LINE+2]]:14 = (#0 + #3)

The gap region appears to start from the 'E' in 'EXPR'. That looks like it's 
too early -- I'd expect it to start after the final closing parenthesis ")".



Comment at: clang/test/CoverageMapping/macro-expressions.cpp:75
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+4]]:10 -> [[@LINE+4]]:12 = (#0 + #5)
+  // CHECK-NEXT: Gap,File 0, [[@LINE+3]]:10 -> [[@LINE+3]]:20 = #5
   // CHECK-NEXT: File 0, [[@LINE+2]]:20 -> [[@LINE+2]]:31 = #5

Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

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

[PATCH] D82502: [PowerPC] Implement Load VSX Vector and Sign Extend and Zero Extend

[Albion Fung via Phabricator via cfe-commits]

Conanap updated this revision to Diff 283061.
Conanap marked 2 inline comments as done.
Conanap added a comment.

Clang formatted relevant lines, combined LE and BE tests as they produced the 
same ASM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82502

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.h
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
@@ -1,13 +1,13 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
 ; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
-; RUN:   FileCheck %s
+; RUN: FileCheck %s
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
 ; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
-; RUN:   FileCheck %s
+; RUN: FileCheck %s
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -O0 \
 ; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
-; RUN:   FileCheck %s --check-prefix=CHECK-O0
+; RUN: FileCheck %s --check-prefix=CHECK-O0
 
 ; These test cases aims to test the builtins for the Power10 VSX vector
 ; instructions introduced in ISA 3.1.
@@ -239,3 +239,181 @@
   store i64 %conv, i64* %add.ptr, align 8
   ret void
 }
+
+define dso_local <1 x i128> @vec_xl_zext(i64 %__offset, i8* nocapture readonly %__pointer) {
+; CHECK-LABEL: vec_xl_zext:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:lxvrbx v2, r4, r3
+; CHECK-NEXT:blr
+;
+; CHECK-O0-LABEL: vec_xl_zext:
+; CHECK-O0:   # %bb.0: # %entry
+; CHECK-O0-NEXT:lxvrbx vs0, r4, r3
+; CHECK-O0-NEXT:xxlor v2, vs0, vs0
+; CHECK-O0-NEXT:blr
+entry:
+  %add.ptr = getelementptr inbounds i8, i8* %__pointer, i64 %__offset
+  %0 = load i8, i8* %add.ptr, align 1
+  %conv = zext i8 %0 to i128
+  %splat.splatinsert = insertelement <1 x i128> undef, i128 %conv, i32 0
+  ret <1 x i128> %splat.splatinsert
+}
+
+define dso_local <1 x i128> @vec_xl_zext_short(i64 %__offset, i16* nocapture readonly %__pointer) {
+; CHECK-LABEL: vec_xl_zext_short:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:sldi r3, r3, 1
+; CHECK-NEXT:lxvrhx v2, r4, r3
+; CHECK-NEXT:blr
+;
+; CHECK-O0-LABEL: vec_xl_zext_short:
+; CHECK-O0:   # %bb.0: # %entry
+; CHECK-O0-NEXT:sldi r3, r3, 1
+; CHECK-O0-NEXT:lxvrhx vs0, r4, r3
+; CHECK-O0-NEXT:xxlor v2, vs0, vs0
+; CHECK-O0-NEXT:blr
+entry:
+  %add.ptr = getelementptr inbounds i16, i16* %__pointer, i64 %__offset
+  %0 = load i16, i16* %add.ptr, align 2
+  %conv = zext i16 %0 to i128
+  %splat.splatinsert = insertelement <1 x i128> undef, i128 %conv, i32 0
+  ret <1 x i128> %splat.splatinsert
+}
+
+define dso_local <1 x i128> @vec_xl_zext_word(i64 %__offset, i32* nocapture readonly %__pointer) {
+; CHECK-LABEL: vec_xl_zext_word:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:sldi r3, r3, 2
+; CHECK-NEXT:lxvrwx v2, r4, r3
+; CHECK-NEXT:blr
+;
+; CHECK-O0-LABEL: vec_xl_zext_word:
+; CHECK-O0:   # %bb.0: # %entry
+; CHECK-O0-NEXT:sldi r3, r3, 2
+; CHECK-O0-NEXT:lxvrwx vs0, r4, r3
+; CHECK-O0-NEXT:xxlor v2, vs0, vs0
+; CHECK-O0-NEXT:blr
+entry:
+  %add.ptr = getelementptr inbounds i32, i32* %__pointer, i64 %__offset
+  %0 = load i32, i32* %add.ptr, align 4
+  %conv = zext i32 %0 to i128
+  %splat.splatinsert = insertelement <1 x i128> undef, i128 %conv, i32 0
+  ret <1 x i128> %splat.splatinsert
+}
+
+define dso_local <1 x i128> @vec_xl_zext_dw(i64 %__offset, i64* nocapture readonly %__pointer) {
+; CHECK-LABEL: vec_xl_zext_dw:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:sldi r3, r3, 3
+; CHECK-NEXT:lxvrdx v2, r4, r3
+; CHECK-NEXT:blr
+;
+; CHECK-O0-LABEL: vec_xl_zext_dw:
+; CHECK-O0:   # %bb.0: # %entry
+; CHECK-O0-NEXT:sldi r3, r3, 3
+; CHECK-O0-NEXT:lxvrdx vs0, r4, r3
+; CHECK-O0-NEXT:xxlor v2, vs0, vs0
+; CHECK-O0-NEXT:blr
+entry:
+  %add.ptr = getelementptr inbounds i64, i64* %__pointer, i64 %__offset
+  %0 = load i64, i64* %add.ptr, align 8
+  %conv = zext i64 %0 to i128
+  %splat.splatinsert = insertelement <1 x i128> undef, i128 %conv, i32 0
+  ret <1 x i128> %splat.splatinsert
+}
+
+define dso_local <1 x i128> @vec_xl_sext_b(i64 %offset, i8* %p) {
+; CHECK-LABEL: vec_xl_sext_b:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:lbzx r3, r4, r3
+; CHECK-NEXT:extsb r3, r3
+; CHECK-NEXT:sradi r4, r3, 63
+; CHECK-NEXT:mtvsrdd v2, r4, r3
+; CHECK-NEXT:blr
+;
+; CHECK-O0-LABEL: vec_xl_sext_b:
+; CHECK-O0: 

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

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

xbolva00 added a comment.

Could this solve https://bugs.llvm.org/show_bug.cgi?id=40921?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85256

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

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Sent D85257 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83273

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

[PATCH] D85174: BPF: simplify IR generation for __builtin_btf_type_id()

[Yonghong Song via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG00602ee7ef0b: BPF: simplify IR generation for 
__builtin_btf_type_id() (authored by yonghong-song).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85174

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtin-bpf-btf-type-id.c
  llvm/include/llvm/IR/IntrinsicsBPF.td
  llvm/lib/Target/BPF/BPFPreserveDIType.cpp
  llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll

Index: llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
===
--- llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
+++ llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
@@ -13,7 +13,7 @@
 ; bpf_log(__builtin_btf_type_id(tmp__abc, 0), __abc, sizeof(tmp__abc));
 ;   }
 ;   void prog2() {
-; bpf_log(__builtin_btf_type_id(__abc, 1), __abc, sizeof(tmp__abc));
+; bpf_log(__builtin_btf_type_id(__abc, 0), __abc, sizeof(tmp__abc));
 ;   }
 ;   void prog3() {
 ; bpf_log(__builtin_btf_type_id(tmp__abc.f1[3], 1), __abc, sizeof(tmp__abc));
@@ -21,25 +21,23 @@
 ; Compilation flag:
 ;   clang -target bpf -O2 -g -S -emit-llvm test.c
 
-%struct.anon = type { [100 x i8], i32 }
-
 @tmp__abc = dso_local global { <{ i8, i8, [98 x i8] }>, i32 } { <{ i8, i8, [98 x i8] }> <{ i8 1, i8 3, [98 x i8] zeroinitializer }>, i32 0 }, align 4, !dbg !0
 
 ; Function Attrs: nounwind
 define dso_local void @prog1() local_unnamed_addr #0 !dbg !28 {
 entry:
-  %0 = tail call i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon* bitcast ({ <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc to %struct.anon*), i32 1, i64 0), !dbg !31, !llvm.preserve.access.index !7
+  %0 = tail call i32 @llvm.bpf.btf.type.id(i32 0, i64 0), !dbg !31, !llvm.preserve.access.index !7
   %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !32
   ret void, !dbg !33
 }
 
 ; Function Attrs: nounwind readnone
-declare i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon*, i32, i64) #1
+declare i32 @llvm.bpf.btf.type.id(i32, i64) #1
 
 ; Function Attrs: nounwind
 define dso_local void @prog2() local_unnamed_addr #0 !dbg !34 {
 entry:
-  %0 = tail call i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon* bitcast ({ <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc to %struct.anon*), i32 0, i64 1), !dbg !35, !llvm.preserve.access.index !6
+  %0 = tail call i32 @llvm.bpf.btf.type.id(i32 1, i64 0), !dbg !35, !llvm.preserve.access.index !6
   %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !36
   ret void, !dbg !37
 }
@@ -47,56 +45,55 @@
 ; Function Attrs: nounwind
 define dso_local void @prog3() local_unnamed_addr #0 !dbg !38 {
 entry:
-  %0 = tail call i32 @llvm.bpf.btf.type.id.p0i8.i32(i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 2, i64 1), i32 1, i64 1), !dbg !39, !llvm.preserve.access.index !11
+  %0 = tail call i32 @llvm.bpf.btf.type.id(i32 2, i64 1), !dbg !39, !llvm.preserve.access.index !11
   %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !40
   ret void, !dbg !41
 }
 
-; CHECK-LABEL:   prog1
-; CHECK: r1 = 3
-; CHECK-LABEL:   prog2
-; CHECK: r1 = 10
-; CHECK-LABEL:   prog3
-; CHECK: r1 = 4
-;
-; CHECK: .long   0   # BTF_KIND_STRUCT(id = 3)
-; CHECK-NEXT:.long   67108866# 0x402
-; CHECK-NEXT:.long   104
-; CHECK-NEXT:.long   13
-; CHECK-NEXT:.long   5
-; CHECK-NEXT:.long   0   # 0x0
-; CHECK-NEXT:.long   16
-; CHECK-NEXT:.long   7
-; CHECK-NEXT:.long   800 # 0x320
-; CHECK-NEXT:.long   19  # BTF_KIND_INT(id = 4)
-; CHECK-NEXT:.long   16777216# 0x100
-; CHECK-NEXT:.long   1
-; CHECK-NEXT:.long   16777224# 0x108
-; CHECK: .long   0   # BTF_KIND_PTR(id = 10)
-; CHECK-NEXT:.long   33554432# 0x200
-; CHECK-NEXT:.long   3
+; CHECK-LABEL:   prog1
+; CHECK: r1 = 3
+; CHECK-LABEL:   prog2
+; CHECK: r1 = 10
+; CHECK-LABEL:   prog3
+; CHECK: r1 = 4
 
-; CHECK: .long   16  # FieldReloc
-; CHECK-NEXT:.long   {{[0-9]+}}  # Field reloc section string 

[clang] 00602ee - BPF: simplify IR generation for __builtin_btf_type_id()

[Yonghong Song via cfe-commits]

Author: Yonghong Song
Date: 2020-08-04T16:29:42-07:00
New Revision: 00602ee7ef0bf6c68d690a2bd729c12b95c95c99

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

LOG: BPF: simplify IR generation for __builtin_btf_type_id()

This patch simplified IR generation for __builtin_btf_type_id().
For __builtin_btf_type_id(obj, flag), previously IR builtin
looks like
   if (obj is a lvalue)
 llvm.bpf.btf.type.id(obj.ptr, 1, flag)  !type
   else
 llvm.bpf.btf.type.id(obj, 0, flag)  !type
The purpose of the 2nd argument is to differentiate
   __builtin_btf_type_id(obj, flag) where obj is a lvalue
vs.
   __builtin_btf_type_id(obj.ptr, flag)

Note that obj or obj.ptr is never used by the backend
and the `obj` argument is only used to derive the type.
This code sequence is subject to potential llvm CSE when
  - obj is the same .e.g., nullptr
  - flag is the same
  - metadata type is different, e.g., typedef of struct "s"
and strust "s".
In the above, we don't want CSE since their metadata is different.

This patch change IR builtin to
   llvm.bpf.btf.type.id(seq_num, flag)  !type
and seq_num is always increasing. This will prevent potential
llvm CSE.

Also report an error if the type name is empty for
remote relocation since remote relocation needs non-empty
type name to do relocation against vmlinux.

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/builtin-bpf-btf-type-id.c
llvm/include/llvm/IR/IntrinsicsBPF.td
llvm/lib/Target/BPF/BPFPreserveDIType.cpp
llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 18911184aa41..1797dfa05234 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -10961,68 +10961,7 @@ Value *CodeGenFunction::EmitBPFBuiltinExpr(unsigned 
BuiltinID,
 {FieldAddr->getType()});
 return Builder.CreateCall(FnGetFieldInfo, {FieldAddr, InfoKind});
   }
-  case BPF::BI__builtin_btf_type_id: {
-Value *FieldVal = nullptr;
-
-// The LValue cannot be converted Value in order to be used as the function
-// parameter. If it is a structure, it is the "alloca" result of the LValue
-// (a pointer) is used in the parameter. If it is a simple type,
-// the value will be loaded from its corresponding "alloca" and used as
-// the parameter. In our case, let us just get a pointer of the LValue
-// since we do not really use the parameter. The purpose of parameter
-// is to prevent the generated IR llvm.bpf.btf.type.id intrinsic call,
-// which carries metadata, from being changed.
-bool IsLValue = E->getArg(0)->isLValue();
-if (IsLValue)
-  FieldVal = EmitLValue(E->getArg(0)).getPointer(*this);
-else
-  FieldVal = EmitScalarExpr(E->getArg(0));
-
-if (!getDebugInfo()) {
-  CGM.Error(E->getExprLoc(), "using __builtin_btf_type_id() without -g");
-  return nullptr;
-}
-
-// Generate debuginfo type for the first argument.
-llvm::DIType *DbgInfo =
-getDebugInfo()->getOrCreateStandaloneType(E->getArg(0)->getType(),
-  E->getArg(0)->getExprLoc());
-
-ConstantInt *Flag = cast(EmitScalarExpr(E->getArg(1)));
-Value *FlagValue = ConstantInt::get(Int64Ty, Flag->getSExtValue());
-
-// Built the IR for the btf_type_id intrinsic.
-//
-// In the above, we converted LValue argument to a pointer to LValue.
-// For example, the following
-//   int v;
-//   C1: __builtin_btf_type_id(v, flag);
-// will be converted to
-//   L1: llvm.bpf.btf.type.id(, flag)
-// This makes it hard to 
diff erentiate from
-//   C2: __builtin_btf_type_id(, flag);
-// to
-//   L2: llvm.bpf.btf.type.id(, flag)
-//
-// If both C1 and C2 are present in the code, the llvm may later
-// on do CSE on L1 and L2, which will result in incorrect tagged types.
-//
-// The C1->L1 transformation only happens if the argument of
-// __builtin_btf_type_id() is a LValue. So Let us put whether
-// the argument is an LValue or not into generated IR. This should
-// prevent potential CSE from causing debuginfo type loss.
-//
-// The generated IR intrinsics will hence look like
-//   L1: llvm.bpf.btf.type.id(, 1, flag) !di_type_for_{v};
-//   L2: llvm.bpf.btf.type.id(, 0, flag) !di_type_for_{};
-Constant *CV = ConstantInt::get(IntTy, IsLValue);
-llvm::Function *FnBtfTypeId = llvm::Intrinsic::getDeclaration(
-(), llvm::Intrinsic::bpf_btf_type_id,
-{FieldVal->getType(), CV->getType()});
-CallInst *Fn = Builder.CreateCall(FnBtfTypeId, {FieldVal, CV, 

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

In D83273#2194842 , @nickdesaulniers 
wrote:

> In D83273#2194832 , @craig.topper 
> wrote:
>
>> It's a pretty nasty revert in our downstream tree where we have prototyped 
>> future ISAs. So I'd like a little time to take a look.
>>
>> @nickdesaulniers what cpu and fetaures are on your command lines. 
>> getImpliedDisabledFeatures should only be called if some feature is 
>> explicitly being disabled.
>
> Likely, as the kernel is very explicit to disable the use of all FP 
> extensions, see 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/Makefile#n57.
>   Maybe we could just set fewer flags, but there's still probably room for 
> improvement here, too.

That said, it's a 10% compile time regression for compiling something like the 
linux kernel or anything that's very explicit what flags they set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83273

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

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added a reviewer: rtrieu.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
rsmith requested review of this revision.

This warning diagnoses cases where an expression is compared to a
constant, and the comparison is tautological due to the form of the
expression (but not merely due to its type). This applies in cases such
as comparisons of bit-fields and the result of bit-masks.

The new warning is added to the Clang diagnostic group
-Wtautological-constant-in-range-compare but not to the
formerly-equivalent GCC-compatibility diagnostic group -Wtype-limits,
which retains its old meaning of diagnosing only tautological
comparisons to extremal values of a type (eg, int > INT_MAX).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85256

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/tautological-constant-compare.c

Index: clang/test/Sema/tautological-constant-compare.c
===
--- clang/test/Sema/tautological-constant-compare.c
+++ clang/test/Sema/tautological-constant-compare.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST -verify %s
@@ -545,9 +545,9 @@
   if (maybe >= e)
   return 0;
 
-  // For the time being, use the declared type of bit-fields rather than their
-  // length when determining whether a value is in-range.
-  // FIXME: Reconsider this.
+  // We only warn on out-of-range bitfields and expressions with limited range
+  // under -Wtantological-in-range-compare, not under -Wtype-limits, because
+  // the warning is not based on the type alone.
   struct A {
 int a : 3;
 unsigned b : 3;
@@ -555,13 +555,50 @@
 unsigned long d : 3;
   } a;
   if (a.a < 3) {}
-  if (a.a < 4) {}
+  if (a.a < 4) {
+  } // #bitfield1
   if (a.b < 7) {}
-  if (a.b < 8) {}
+  if (a.b < 8) {
+  } // #bitfield2
   if (a.c < 3) {}
-  if (a.c < 4) {}
+  if (a.c < 4) {
+  } // #bitfield3
   if (a.d < 7) {}
-  if (a.d < 8) {}
+  if (a.d < 8) {
+  } // #bitfield4
+#if TEST == 2
+  // expected-warning@#bitfield1 {{comparison of 3-bit signed value < 4 is always true}}
+  // expected-warning@#bitfield2 {{comparison of 3-bit unsigned value < 8 is always true}}
+  // expected-warning@#bitfield3 {{comparison of 3-bit signed value < 4 is always true}}
+  // expected-warning@#bitfield4 {{comparison of 3-bit unsigned value < 8 is always true}}
+#endif
+
+  if ((s & 0xff) < 0) {
+  } // #valuerange1
+  if ((s & 0xff) < 1) {
+  }
+  if ((s & -3) < -4) {
+  } // #valuerange2
+  if ((s & -3) < -3) {
+  }
+  if ((s & -3) < 4u) {
+  } // (true if s non-negative)
+  if ((s & -3) > 4u) {
+  } // (true if s negative)
+  if ((s & -3) == 4u) {
+  } // #valuerange3 (never true)
+  if ((s & -3) == 3u) {
+  }
+  if ((s & -3) == -5u) {
+  } // #valuerange4
+  if ((s & -3) == -4u) {
+  }
+#if TEST == 2
+  // expected-warning@#valuerange1 {{comparison of 8-bit unsigned value < 0 is always false}}
+  // expected-warning@#valuerange2 {{comparison of 3-bit signed value < -4 is always false}}
+  // expected-warning@#valuerange3 {{comparison of 3-bit signed value == 4 is always false}}
+  // expected-warning@#valuerange4 {{comparison of 3-bit signed value == 4294967291 is always false}}
+#endif
 
   return 1;
 }
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10792,15 +10792,16 @@
   S.Context.hasSameUnqualifiedType(Constant->getType(), Other->getType()))
 return false;
 
-  // TODO: Investigate using GetExprRange() to get tighter bounds
-  // on the bit ranges.
+  IntRange OtherValueRange =
+  GetExprRange(S.Context, Other, S.isConstantEvaluated());
+
   QualType OtherT = Other->getType();
   if (const auto *AT = OtherT->getAs())
 OtherT = AT->getValueType();
-  IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
+  IntRange OtherTypeRange = IntRange::forValueOfType(S.Context, OtherT);
 
   // Special case for ObjC BOOL on targets where its a typedef for a signed char
-  // 

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

I can optimize getImpliedDisabledFeatures & getImpliedEnabledFeatures


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83273

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

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D83273#2194832 , @craig.topper 
wrote:

> It's a pretty nasty revert in our downstream tree where we have prototyped 
> future ISAs. So I'd like a little time to take a look.
>
> @nickdesaulniers what cpu and fetaures are on your command lines. 
> getImpliedDisabledFeatures should only be called if some feature is 
> explicitly being disabled.

Likely, as the kernel is very explicit to disable the use of all FP extensions, 
see 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/Makefile#n57.
  Maybe we could just set fewer flags, but there's still probably room for 
improvement here, too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83273

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

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

It's a pretty nasty revert in our downstream tree where we have prototyped 
future ISAs. So I'd like a little time to take a look.

@nickdesaulniers what cpu and fetaures are on your command lines. 
getImpliedDisabledFeatures should only be called if some feature is explicitly 
being disabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83273

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

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a subscriber: nikic.
nickdesaulniers added a comment.

cc @nikic 
http://llvm-compile-time-tracker.com/compare.php?from=094e99d264c937cad33796b8c92fe123cb544c9e=16f3d698f2afbbea43e0c3df81df6f2a640ce806=instructions
 gives me an error, am I holding it wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83273

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

Re: [PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Eric Christopher via cfe-commits]

Ow. Can revert and reapply after we fix the caching problem perhaps?

-eric

On Tue, Aug 4, 2020 at 3:48 PM Nick Desaulniers via Phabricator <
revi...@reviews.llvm.org> wrote:

> nickdesaulniers added a comment.
>
> I just collected a perf profile for an entire build.
> `getImpliedDisabledFeatures` is at the top with 9.15% of time spent in self.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D83273/new/
>
> https://reviews.llvm.org/D83273
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

I just collected a perf profile for an entire build.  
`getImpliedDisabledFeatures` is at the top with 9.15% of time spent in self.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83273

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

[PATCH] D85253: [clangd] Show correct hover tooltip for non-preamble macro definition.

[Ilya Golovenko via Phabricator via cfe-commits]

ilya-golovenko added reviewers: kadircet, sammccall.
ilya-golovenko added a comment.

I'm not quite sure my fix is the best way to fix the issue, so any advices are 
appreciated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85253

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

[PATCH] D85253: [clangd] Show correct hover tooltip for non-preamble macro definition.

[Ilya Golovenko via Phabricator via cfe-commits]

ilya-golovenko created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
ilya-golovenko requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Incorrect definition is shown in tooltip when hovering over a non-preamble 
macro definition.
In the example below the definition will have 'namespace ns {}' instead of 
correct '#define FOO 1'.

namespace ns {
#define FOO 1
}


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85253

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1308,6 +1308,17 @@
 HI.Kind = index::SymbolKind::Macro;
 HI.Definition = "#define MACRO 0";
   }},
+  {
+  R"cpp(// Macro
+namespace {
+#define [[MA^CRO]] 0
+}
+  )cpp",
+  [](HoverInfo ) {
+HI.Name = "MACRO";
+HI.Kind = index::SymbolKind::Macro;
+HI.Definition = "#define MACRO 0";
+  }},
   {
   R"cpp(// Macro
 #define MACRO 0
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -980,6 +980,12 @@
   if (SM.getLocForStartOfFile(SM.getFileID(Loc)) != Loc)
 Loc = Loc.getLocWithOffset(-1);
   MacroDefinition MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc);
+  // If the definition was not found before the current token location then
+  // it is possible the current token is a macro definition itself.
+  if (!MacroDef) {
+Loc = SpelledTok.location().getLocWithOffset(1);
+MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc);
+  }
   if (auto *MI = MacroDef.getMacroInfo())
 return DefinedMacro{
 IdentifierInfo->getName(), MI,


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1308,6 +1308,17 @@
 HI.Kind = index::SymbolKind::Macro;
 HI.Definition = "#define MACRO 0";
   }},
+  {
+  R"cpp(// Macro
+namespace {
+#define [[MA^CRO]] 0
+}
+  )cpp",
+  [](HoverInfo ) {
+HI.Name = "MACRO";
+HI.Kind = index::SymbolKind::Macro;
+HI.Definition = "#define MACRO 0";
+  }},
   {
   R"cpp(// Macro
 #define MACRO 0
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -980,6 +980,12 @@
   if (SM.getLocForStartOfFile(SM.getFileID(Loc)) != Loc)
 Loc = Loc.getLocWithOffset(-1);
   MacroDefinition MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc);
+  // If the definition was not found before the current token location then
+  // it is possible the current token is a macro definition itself.
+  if (!MacroDef) {
+Loc = SpelledTok.location().getLocWithOffset(1);
+MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc);
+  }
   if (auto *MI = MacroDef.getMacroInfo())
 return DefinedMacro{
 IdentifierInfo->getName(), MI,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85252: [Driver] Accept -fno-lto in clang-cl

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks created this revision.
aeubanks added reviewers: hans, thakis.
Herald added subscribers: cfe-commits, dang, dexonsmith, inglorion.
Herald added a project: clang.
aeubanks requested review of this revision.

Some compiler-rt tests check for the presence of the compiler accepting
-fno-lto to add that flag. Otherwise some tests don't link due to
-flto mismatch between compiling and linking.

$ cmake ... -DLLVM_ENABLE_LTO=Thin ...
$ ninja 
projects/compiler-rt/lib/sanitizer_common/tests/Sanitizer-x86_64-Test.exe
previously failed, now links.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85252

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -586,6 +586,9 @@
 // RUN: %clang_cl -### /c -flto -- %s 2>&1 | FileCheck -check-prefix=LTO %s
 // LTO: -flto
 
+// RUN: %clang_cl -### /c -flto -fno-lto -- %s 2>&1 | FileCheck 
-check-prefix=LTO-NO %s
+// LTO-NO-NOT: "-flto"
+
 // RUN: %clang_cl -### /c -flto=thin -- %s 2>&1 | FileCheck 
-check-prefix=LTO-THIN %s
 // LTO-THIN: -flto=thin
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1351,7 +1351,7 @@
   HelpText<"Set LTO mode to either 'full' or 'thin'">, Values<"thin,full">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Enable LTO in 'full' mode">;
-def fno_lto : Flag<["-"], "fno-lto">, Group,
+def fno_lto : Flag<["-"], "fno-lto">, Flags<[CoreOption, CC1Option]>, 
Group,
   HelpText<"Disable LTO mode (default)">;
 def flto_jobs_EQ : Joined<["-"], "flto-jobs=">,
   Flags<[CC1Option]>, Group,


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -586,6 +586,9 @@
 // RUN: %clang_cl -### /c -flto -- %s 2>&1 | FileCheck -check-prefix=LTO %s
 // LTO: -flto
 
+// RUN: %clang_cl -### /c -flto -fno-lto -- %s 2>&1 | FileCheck -check-prefix=LTO-NO %s
+// LTO-NO-NOT: "-flto"
+
 // RUN: %clang_cl -### /c -flto=thin -- %s 2>&1 | FileCheck -check-prefix=LTO-THIN %s
 // LTO-THIN: -flto=thin
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1351,7 +1351,7 @@
   HelpText<"Set LTO mode to either 'full' or 'thin'">, Values<"thin,full">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Enable LTO in 'full' mode">;
-def fno_lto : Flag<["-"], "fno-lto">, Group,
+def fno_lto : Flag<["-"], "fno-lto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Disable LTO mode (default)">;
 def flto_jobs_EQ : Joined<["-"], "flto-jobs=">,
   Flags<[CC1Option]>, Group,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added subscribers: Nathan-Huckleberry, nickdesaulniers.
nickdesaulniers added a comment.

Hi @craig.topper , @Nathan-Huckleberry and I are seeing 
`getImpliedEnabledFeatures` at the top of our profiles now.  It seems that 
`llvm::X86::getImpliedFeatures` is repeatedly queried for the same inputs.  
Should we try to cache the `ImpliedBits` query or try to have 
`X86TargetInfo::setFeatureEnabled` not rebuild `ImpliedBits` every invocation?  
FWIW, I'm seeing `getImpliedEnabledFeatures` take 2.33% of compilation for the 
median duration to build input file from the x86 Linux kernel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83273

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

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

This file is auto generated. The real documentation should go to 
`clang/include/clang/Driver/Options.td`


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

https://reviews.llvm.org/D85239

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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 283028.
yaxunl marked 2 inline comments as done.
yaxunl added a comment.

revised by Artem's comments.


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

https://reviews.llvm.org/D71726

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCUDA/amdgpu-atomic-ops.cu
  clang/test/CodeGenOpenCL/atomic-ops.cl
  clang/test/Sema/atomic-ops.c
  clang/test/SemaCUDA/amdgpu-atomic-ops.cu
  clang/test/SemaOpenCL/atomic-ops.cl

Index: clang/test/SemaOpenCL/atomic-ops.cl
===
--- clang/test/SemaOpenCL/atomic-ops.cl
+++ clang/test/SemaOpenCL/atomic-ops.cl
@@ -1,10 +1,13 @@
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only -triple=spir64
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only -triple=amdgcn-amdhsa-amd-opencl
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify=expected,spir \
+// RUN:   -fsyntax-only -triple=spir64
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only \
+// RUN:   -triple=amdgcn-amd-amdhsa
 
 // Basic parsing/Sema tests for __opencl_atomic_*
 
 #pragma OPENCL EXTENSION cl_khr_int64_base_atomics : enable
 #pragma OPENCL EXTENSION cl_khr_int64_extended_atomics : enable
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
 
 typedef __INTPTR_TYPE__ intptr_t;
 typedef int int8 __attribute__((ext_vector_type(8)));
@@ -36,7 +39,7 @@
 
 atomic_int gn;
 void f(atomic_int *i, const atomic_int *ci,
-   atomic_intptr_t *p, atomic_float *d,
+   atomic_intptr_t *p, atomic_float *f, atomic_double *d, atomic_half *h, // expected-error {{unknown type name 'atomic_half'}}
int *I, const int *CI,
intptr_t *P, float *D, struct S *s1, struct S *s2,
global atomic_int *i_g, local atomic_int *i_l, private atomic_int *i_p,
@@ -57,37 +60,38 @@
 
   __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_load(p, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_load(d, memory_order_seq_cst, memory_scope_work_group);
+  __opencl_atomic_load(f, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_load(ci, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_load(i_c, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to non-constant _Atomic type ('__constant atomic_int *' (aka '__constant _Atomic(int) *') invalid)}}
 
   __opencl_atomic_store(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_store(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  (int)__opencl_atomic_store(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{operand of type 'void' where arithmetic or pointer type is required}}
+  (int)__opencl_atomic_store(f, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{operand of type 'void' where arithmetic or pointer type is required}}
 
   int exchange_1 = __opencl_atomic_exchange(i, 1, memory_order_seq_cst, memory_scope_work_group);
   int exchange_2 = __opencl_atomic_exchange(I, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to _Atomic}}
 
   __opencl_atomic_fetch_add(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_add(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_add(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_add(f, 1.0f, memory_order_seq_cst, memory_scope_work_group);
+  __opencl_atomic_fetch_add(d, 1.0, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_and(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_and(f, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
 
   __opencl_atomic_fetch_min(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_max(i, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_min(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic 

[PATCH] D85247: [Darwin] [Driver] Clang should invoke dsymutil for multiarch builds

[Daniel Sanders via Phabricator via cfe-commits]

dsanders created this revision.
dsanders added reviewers: bogner, beanz, JDevlieghere.
Herald added subscribers: cfe-commits, dexonsmith, aprantl.
Herald added a project: clang.
dsanders requested review of this revision.
Herald added a subscriber: ormris.

Much like with debug info with LTO, this case also uses temporary
object files and therefore needs to externalize it before the
temporary files are deleted

Depends on D84565 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85247

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/test/Driver/darwin-dsymutil.c

Index: clang/test/Driver/darwin-dsymutil.c
===
--- clang/test/Driver/darwin-dsymutil.c
+++ clang/test/Driver/darwin-dsymutil.c
@@ -61,3 +61,14 @@
 
 // CHECK-DSYMUTIL-LTO: "/usr/bin/ld" "-demangle" "-object_path_lto"
 // CHECK-DSYMUTIL-LTO: "/usr/bin/dsymutil" "-o" "a.out.dSYM" "a.out"
+
+// Check that clang provides the linker with a temporary object file path and
+// runs dsymutil when building for multiple architectures.
+// RUN: touch %t.o
+// RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 -arch arm64 %t.o -g \
+// RUN: -flto -### 2> %t
+// RUN: FileCheck -check-prefix=CHECK-DSYMUTIL-MULTIARCH < %t %s
+
+// CHECK-DSYMUTIL-MULTIARCH: "/usr/bin/ld" "-demangle" "-object_path_lto"
+// CHECK-DSYMUTIL-MULTIARCH: "/usr/bin/ld" "-demangle" "-object_path_lto"
+// CHECK-DSYMUTIL-MULTIARCH: "/usr/bin/dsymutil" "-o" "a.out.dSYM" "a.out"
Index: clang/lib/Driver/ToolChains/Darwin.h
===
--- clang/lib/Driver/ToolChains/Darwin.h
+++ clang/lib/Driver/ToolChains/Darwin.h
@@ -59,10 +59,10 @@
 };
 
 class LLVM_LIBRARY_VISIBILITY Linker : public MachOTool {
-  bool NeedsTempPath(const InputInfoList ,
+  bool NeedsTempPath(const InputInfo , const InputInfoList ,
  const llvm::opt::ArgList ) const;
   void AddLinkArgs(Compilation , const llvm::opt::ArgList ,
-   llvm::opt::ArgStringList ,
+   llvm::opt::ArgStringList , const InputInfo ,
const InputInfoList , unsigned Version[5]) const;
 
 public:
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -167,7 +167,8 @@
 CmdArgs.push_back("-force_cpusubtype_ALL");
 }
 
-bool darwin::Linker::NeedsTempPath(const InputInfoList ,
+bool darwin::Linker::NeedsTempPath(const InputInfo ,
+   const InputInfoList ,
const ArgList ) const {
   Arg *A = Args.getLastArg(options::OPT_g_Group);
   if (A && !A->getOption().matches(options::OPT_g0) &&
@@ -181,6 +182,11 @@
 if (Input.getType() != types::TY_Object)
   return true;
 
+  // If we're going to use lipo (i.e. the linker output is a temporary file too)
+  // then we'll need to run dsymutil after linking too
+  if (Inputs.size() != 1 && Output.getType() != types::TY_Nothing)
+return true;
+
   return false;
 }
 
@@ -209,6 +215,7 @@
 
 void darwin::Linker::AddLinkArgs(Compilation , const ArgList ,
  ArgStringList ,
+ const InputInfo ,
  const InputInfoList ,
  unsigned Version[5]) const {
   const Driver  = getToolChain().getDriver();
@@ -228,7 +235,8 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs, Args)) {
+  if (D.isUsingLTO() && Version[0] >= 116 &&
+  NeedsTempPath(Output, Inputs, Args)) {
 std::string TmpPathName;
 if (D.getLTOMode() == LTOK_Full) {
   // If we are using full LTO, then automatically create a temporary file
@@ -541,7 +549,7 @@
 
   // I'm not sure why this particular decomposition exists in gcc, but
   // we follow suite for ease of comparison.
-  AddLinkArgs(C, Args, CmdArgs, Inputs, Version);
+  AddLinkArgs(C, Args, CmdArgs, Output, Inputs, Version);
 
   if (willEmitRemarks(Args) &&
   checkRemarksOptions(getToolChain().getDriver(), Args,
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2024,7 +2024,8 @@
 
 // Lipo if necessary, we do it this way because we need to set the arch flag
 // so that -Xarch_ gets overwritten.
-if (Inputs.size() == 1 || Act->getType() == types::TY_Nothing)
+bool LipoNeeded = Inputs.size() != 1 && Act->getType() != types::TY_Nothing;
+if (!LipoNeeded)
   Actions.append(Inputs.begin(), Inputs.end());
 else
   

Re: [PATCH] D85231: Protect against filenames with no extension at all.

[Eric Christopher via cfe-commits]

Could maybe add an assert along with the patch as well as an assert only
test?

On Tue, Aug 4, 2020, 1:42 PM Sterling Augustine via Phabricator <
revi...@reviews.llvm.org> wrote:

> saugustine added a comment.
>
> The darwin-dsymutil.c tests this code path right now on line 33. I found
> this error by running it under asan; otherwise it was fully latent.
>
> It seems pretty clear to me that when End == -1 (StringRef::npos),
> creating a string ref from that is obviously a bug.
>
> I'm not even sure how to check for a failure. clang will almost never
> crash; there is just a very subtle bounds violation or empty stringref. How
> it manifests itself depends on subsequent usage of the stringref, and stack
> layout.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D85231/new/
>
> https://reviews.llvm.org/D85231
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85231: Protect against filenames with no extension at all.

[Sterling Augustine via Phabricator via cfe-commits]

saugustine added a comment.

The darwin-dsymutil.c tests this code path right now on line 33. I found this 
error by running it under asan; otherwise it was fully latent.

It seems pretty clear to me that when End == -1 (StringRef::npos), creating a 
string ref from that is obviously a bug.

I'm not even sure how to check for a failure. clang will almost never crash; 
there is just a very subtle bounds violation or empty stringref. How it 
manifests itself depends on subsequent usage of the stringref, and stack layout.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85231

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

[PATCH] D85185: [SyntaxTree] Add test coverage for `->*` operator

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:2330
+int X::*pmi;
+void test(X x, X y, X* xp) {
   x = y;

Could you add `pmi` as a function parameter (for consistency)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85185

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

[PATCH] D85231: Protect against filenames with no extension at all.

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

Can you add a test that exercises this code path?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85231

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

[PATCH] D84534: [AIX] Static init frontend recovery and backend support

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1865
+if (isSpecialLLVMGlobalArrayForStaticInit()) {
+  if (GlobalUniqueModuleId.empty()) {
+GlobalUniqueModuleId = getUniqueModuleId();

jasonliu wrote:
> We will need to move this part of the if statement to the overrided 
> `emitXXStructorList` as well. (If we think overriding `emitXXStructorList` 
> and calling `emitSpecialLLVMGlobal ` directly is a good idea.)
`getUniqueModuleId` takes `Module` as a parameter, if we want to invoke this 
function inside of `emitXXStructorList` through `emitSpecialLLVMGlobal`, we 
have to change the interface of them, which seems not ideal.


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

https://reviews.llvm.org/D84534

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

[PATCH] D85074: [WebAssembly] Use "signed char" instead of "char" in SIMD intrinsics.

[sunfishcode 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 rG47f7174ffa71: [WebAssembly] Use signed char 
instead of char in SIMD intrinsics. (authored by sunfishcode).

Changed prior to commit:
  https://reviews.llvm.org/D85074?vs=282608=283004#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85074

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/Headers/wasm_simd128.h
  clang/test/CodeGen/builtins-wasm.c

Index: clang/test/CodeGen/builtins-wasm.c
===
--- clang/test/CodeGen/builtins-wasm.c
+++ clang/test/CodeGen/builtins-wasm.c
@@ -3,7 +3,7 @@
 // RUN: not %clang_cc1 -triple wasm64-unknown-unknown -target-feature +nontrapping-fptoint -target-feature +exception-handling -target-feature +bulk-memory -target-feature +atomics -flax-vector-conversions=none -O3 -emit-llvm -o - %s 2>&1 | FileCheck %s -check-prefixes MISSING-SIMD
 
 // SIMD convenience types
-typedef char i8x16 __attribute((vector_size(16)));
+typedef signed char i8x16 __attribute((vector_size(16)));
 typedef short i16x8 __attribute((vector_size(16)));
 typedef int i32x4 __attribute((vector_size(16)));
 typedef long long i64x2 __attribute((vector_size(16)));
@@ -201,7 +201,7 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-int extract_lane_u_i8x16(i8x16 v) {
+int extract_lane_u_i8x16(u8x16 v) {
   return __builtin_wasm_extract_lane_u_i8x16(v, 13);
   // WEBASSEMBLY: extractelement <16 x i8> %v, i32 13
   // WEBASSEMBLY-NEXT: zext
@@ -215,7 +215,7 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-int extract_lane_u_i16x8(i16x8 v) {
+int extract_lane_u_i16x8(u16x8 v) {
   return __builtin_wasm_extract_lane_u_i16x8(v, 7);
   // WEBASSEMBLY: extractelement <8 x i16> %v, i32 7
   // WEBASSEMBLY-NEXT: zext
@@ -291,7 +291,7 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-i8x16 add_saturate_u_i8x16(i8x16 x, i8x16 y) {
+u8x16 add_saturate_u_i8x16(u8x16 x, u8x16 y) {
   return __builtin_wasm_add_saturate_u_i8x16(x, y);
   // WEBASSEMBLY: call <16 x i8> @llvm.uadd.sat.v16i8(
   // WEBASSEMBLY-SAME: <16 x i8> %x, <16 x i8> %y)
@@ -305,7 +305,7 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-i16x8 add_saturate_u_i16x8(i16x8 x, i16x8 y) {
+u16x8 add_saturate_u_i16x8(u16x8 x, u16x8 y) {
   return __builtin_wasm_add_saturate_u_i16x8(x, y);
   // WEBASSEMBLY: call <8 x i16> @llvm.uadd.sat.v8i16(
   // WEBASSEMBLY-SAME: <8 x i16> %x, <8 x i16> %y)
@@ -319,7 +319,7 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-i8x16 sub_saturate_u_i8x16(i8x16 x, i8x16 y) {
+u8x16 sub_saturate_u_i8x16(u8x16 x, u8x16 y) {
   return __builtin_wasm_sub_saturate_u_i8x16(x, y);
   // WEBASSEMBLY: call <16 x i8> @llvm.wasm.sub.saturate.unsigned.v16i8(
   // WEBASSEMBLY-SAME: <16 x i8> %x, <16 x i8> %y)
@@ -357,7 +357,7 @@
   // WEBASSEMBLY-NEXT: ret <16 x i8> %1
 }
 
-i8x16 min_u_i8x16(i8x16 x, i8x16 y) {
+u8x16 min_u_i8x16(u8x16 x, u8x16 y) {
   return __builtin_wasm_min_u_i8x16(x, y);
   // WEBASSEMBLY: %0 = icmp ult <16 x i8> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <16 x i1> %0, <16 x i8> %x, <16 x i8> %y
@@ -371,7 +371,7 @@
   // WEBASSEMBLY-NEXT: ret <16 x i8> %1
 }
 
-i8x16 max_u_i8x16(i8x16 x, i8x16 y) {
+u8x16 max_u_i8x16(u8x16 x, u8x16 y) {
   return __builtin_wasm_max_u_i8x16(x, y);
   // WEBASSEMBLY: %0 = icmp ugt <16 x i8> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <16 x i1> %0, <16 x i8> %x, <16 x i8> %y
@@ -385,7 +385,7 @@
   // WEBASSEMBLY-NEXT: ret <8 x i16> %1
 }
 
-i16x8 min_u_i16x8(i16x8 x, i16x8 y) {
+u16x8 min_u_i16x8(u16x8 x, u16x8 y) {
   return __builtin_wasm_min_u_i16x8(x, y);
   // WEBASSEMBLY: %0 = icmp ult <8 x i16> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <8 x i1> %0, <8 x i16> %x, <8 x i16> %y
@@ -399,7 +399,7 @@
   // WEBASSEMBLY-NEXT: ret <8 x i16> %1
 }
 
-i16x8 max_u_i16x8(i16x8 x, i16x8 y) {
+u16x8 max_u_i16x8(u16x8 x, u16x8 y) {
   return __builtin_wasm_max_u_i16x8(x, y);
   // WEBASSEMBLY: %0 = icmp ugt <8 x i16> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <8 x i1> %0, <8 x i16> %x, <8 x i16> %y
@@ -413,7 +413,7 @@
   // WEBASSEMBLY-NEXT: ret <4 x i32> %1
 }
 
-i32x4 min_u_i32x4(i32x4 x, i32x4 y) {
+u32x4 min_u_i32x4(u32x4 x, u32x4 y) {
   return __builtin_wasm_min_u_i32x4(x, y);
   // WEBASSEMBLY: %0 = icmp ult <4 x i32> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <4 x i1> %0, <4 x i32> %x, <4 x i32> %y
@@ -427,7 +427,7 @@
   // WEBASSEMBLY-NEXT: ret <4 x i32> %1
 }
 
-i32x4 max_u_i32x4(i32x4 x, i32x4 y) {
+u32x4 max_u_i32x4(u32x4 x, u32x4 y) {
   return __builtin_wasm_max_u_i32x4(x, y);
   // WEBASSEMBLY: %0 = icmp ugt <4 x i32> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <4 x i1> %0, <4 x i32> %x, <4 x i32> %y
@@ -441,21 +441,21 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-i16x8 sub_saturate_u_i16x8(i16x8 x, i16x8 y) {
+u16x8 sub_saturate_u_i16x8(u16x8 x, u16x8 y) {
   return __builtin_wasm_sub_saturate_u_i16x8(x, y);
   // WEBASSEMBLY: call <8 x i16> 

[clang] 47f7174 - [WebAssembly] Use "signed char" instead of "char" in SIMD intrinsics.

[Dan Gohman via cfe-commits]

Author: Dan Gohman
Date: 2020-08-04T12:48:40-07:00
New Revision: 47f7174ffa71d339c1a65d1dd9a2ac5ff2abc95d

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

LOG: [WebAssembly] Use "signed char" instead of "char" in SIMD intrinsics.

This allows people to use `int8_t` instead of `char`, -funsigned-char,
and generally decouples SIMD from the specialness of `char`.

And it makes intrinsics like `__builtin_wasm_add_saturate_s_i8x16`
and `__builtin_wasm_add_saturate_u_i8x16` use signed and unsigned
element types, respectively.

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsWebAssembly.def
clang/lib/Headers/wasm_simd128.h
clang/test/CodeGen/builtins-wasm.c

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsWebAssembly.def 
b/clang/include/clang/Basic/BuiltinsWebAssembly.def
index 39f29740cf56d..c0ac74686cf1c 100644
--- a/clang/include/clang/Basic/BuiltinsWebAssembly.def
+++ b/clang/include/clang/Basic/BuiltinsWebAssembly.def
@@ -66,67 +66,67 @@ TARGET_BUILTIN(__builtin_wasm_trunc_saturate_s_i64_f64, 
"LLid", "nc", "nontrappi
 TARGET_BUILTIN(__builtin_wasm_trunc_saturate_u_i64_f64, "LLid", "nc", 
"nontrapping-fptoint")
 
 // SIMD builtins
-TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16cV16cV16c", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16ScV16ScV16Sc", "nc", 
"simd128")
 
-TARGET_BUILTIN(__builtin_wasm_extract_lane_s_i8x16, "iV16cIi", "nc", "simd128")
-TARGET_BUILTIN(__builtin_wasm_extract_lane_u_i8x16, "iV16cIi", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_extract_lane_s_i8x16, "iV16ScIi", "nc", 
"simd128")
+TARGET_BUILTIN(__builtin_wasm_extract_lane_u_i8x16, "iV16UcIUi", "nc", 
"simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_s_i16x8, "iV8sIi", "nc", "simd128")
-TARGET_BUILTIN(__builtin_wasm_extract_lane_u_i16x8, "iV8sIi", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_extract_lane_u_i16x8, "iV8UsIUi", "nc", 
"simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_i32x4, "iV4iIi", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_i64x2, "LLiV2LLiIi", "nc", 
"simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_f32x4, "fV4fIi", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_f64x2, "dV2dIi", "nc", "simd128")
 
-TARGET_BUILTIN(__builtin_wasm_replace_lane_i8x16, "V16cV16cIii", "nc", 
"simd128")
+TARGET_BUILTIN(__builtin_wasm_replace_lane_i8x16, "V16ScV16ScIii", "nc", 
"simd128")
 TARGET_BUILTIN(__builtin_wasm_replace_lane_i16x8, "V8sV8sIii", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_replace_lane_i32x4, "V4iV4iIii", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_replace_lane_i64x2, "V2LLiV2LLiIiLLi", "nc", 
"simd128")
 TARGET_BUILTIN(__builtin_wasm_replace_lane_f32x4, "V4fV4fIif", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_replace_lane_f64x2, "V2dV2dIid", "nc", "simd128")
 
-TARGET_BUILTIN(__builtin_wasm_add_saturate_s_i8x16, "V16cV16cV16c", "nc", 
"simd128")
-TARGET_BUILTIN(__builtin_wasm_add_saturate_u_i8x16, "V16cV16cV16c", "nc", 
"simd128")
+TARGET_BUILTIN(__builtin_wasm_add_saturate_s_i8x16, "V16ScV16ScV16Sc", "nc", 
"simd128")
+TARGET_BUILTIN(__builtin_wasm_add_saturate_u_i8x16, "V16UcV16UcV16Uc", "nc", 
"simd128")
 TARGET_BUILTIN(__builtin_wasm_add_saturate_s_i16x8, "V8sV8sV8s", "nc", 
"simd128")
-TARGET_BUILTIN(__builtin_wasm_add_saturate_u_i16x8, "V8sV8sV8s", "nc", 
"simd128")
+TARGET_BUILTIN(__builtin_wasm_add_saturate_u_i16x8, "V8UsV8UsV8Us", "nc", 
"simd128")
 
-TARGET_BUILTIN(__builtin_wasm_sub_saturate_s_i8x16, "V16cV16cV16c", "nc", 
"simd128")
-TARGET_BUILTIN(__builtin_wasm_sub_saturate_u_i8x16, "V16cV16cV16c", "nc", 
"simd128")
+TARGET_BUILTIN(__builtin_wasm_sub_saturate_s_i8x16, "V16ScV16ScV16Sc", "nc", 
"simd128")
+TARGET_BUILTIN(__builtin_wasm_sub_saturate_u_i8x16, "V16UcV16UcV16Uc", "nc", 
"simd128")
 TARGET_BUILTIN(__builtin_wasm_sub_saturate_s_i16x8, "V8sV8sV8s", "nc", 
"simd128")
-TARGET_BUILTIN(__builtin_wasm_sub_saturate_u_i16x8, "V8sV8sV8s", "nc", 
"simd128")
+TARGET_BUILTIN(__builtin_wasm_sub_saturate_u_i16x8, "V8UsV8UsV8Us", "nc", 
"simd128")
 
-TARGET_BUILTIN(__builtin_wasm_abs_i8x16, "V16cV16c", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_abs_i8x16, "V16ScV16Sc", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_abs_i16x8, "V8sV8s", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_abs_i32x4, "V4iV4i", "nc", "simd128")
 
-TARGET_BUILTIN(__builtin_wasm_min_s_i8x16, "V16cV16cV16c", "nc", "simd128")
-TARGET_BUILTIN(__builtin_wasm_min_u_i8x16, "V16cV16cV16c", "nc", "simd128")
-TARGET_BUILTIN(__builtin_wasm_max_s_i8x16, "V16cV16cV16c", "nc", "simd128")
-TARGET_BUILTIN(__builtin_wasm_max_u_i8x16, "V16cV16cV16c", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_min_s_i8x16, "V16ScV16ScV16Sc", "nc", 

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Looks okay (one grammar nit), probably worth waiting for someone else to chime 
in.




Comment at: clang/docs/ClangCommandLineReference.rst:2139
 
-Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable. A function with a stack protector has a 
guard value added to the stack frame that is checked on function exit. The 
guard value must be positioned in the stack frame such that a buffer overflow 
from a vulnerable variable will overwrite to the guard value before overwriting 
the function's return address. The reference stack guard value is stored in a 
global variable.
 

"overwrite to the guard variable" -> "overwrite the guard variable"


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

https://reviews.llvm.org/D85239

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

[PATCH] D84780: Setting the /bigobj option globally for Windows debug build. https://bugs.llvm.org/show_bug.cgi?id=46733

[Lei Zhang via Phabricator via cfe-commits]

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

LGTM for SPIR-V side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84780

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

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Peter Smith via Phabricator via cfe-commits]

psmith created this revision.
psmith added reviewers: jfb, Shayne, emaste, kristof.beyls, mattdr, ojhunt, 
probinson, reames, serge-sans-paille, dim.
Herald added a subscriber: dexonsmith.
psmith requested review of this revision.

The Clang -fstack-protector documentation mentions what functions are 
considered vulnerable but does not mention the details of the
implementation such as the use of a global variable for the guard value. This 
brings the documentation more in line with the GCC
documentation at: 
https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html and gives 
someone using the option more idea about what is protected.

This partly addresses https://bugs.llvm.org/show_bug.cgi?id=42764


https://reviews.llvm.org/D85239

Files:
  clang/docs/ClangCommandLineReference.rst


Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2136,7 +2136,7 @@
 
 .. option:: -fstack-protector, -fno-stack-protector
 
-Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable. A function with a stack protector has a 
guard value added to the stack frame that is checked on function exit. The 
guard value must be positioned in the stack frame such that a buffer overflow 
from a vulnerable variable will overwrite to the guard value before overwriting 
the function's return address. The reference stack guard value is stored in a 
global variable.
 
 .. option:: -fstack-protector-all
 


Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2136,7 +2136,7 @@
 
 .. option:: -fstack-protector, -fno-stack-protector
 
-Enable stack protectors for some functions vulnerable to stack smashing. This uses a loose heuristic which considers functions vulnerable if they contain a char (or 8bit integer) array or constant sized calls to alloca, which are of greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This uses a loose heuristic which considers functions vulnerable if they contain a char (or 8bit integer) array or constant sized calls to alloca, which are of greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls to alloca are considered vulnerable. A function with a stack protector has a guard value added to the stack frame that is checked on function exit. The guard value must be positioned in the stack frame such that a buffer overflow from a vulnerable variable will overwrite to the guard value before overwriting the function's return address. The reference stack guard value is stored in a global variable.
 
 .. option:: -fstack-protector-all
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85236: [CUDA] Work around a bug in rint() caused by a broken implementation provided by CUDA.

[Justin Lebar via Phabricator via cfe-commits]

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

LGTM, and can we write a test in the test-suite?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85236

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

[PATCH] D85118: [clang][AArch64] Correct return type of Neon vqmovun intrinsics

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Probably the simplest way to verify return types would be using decltype.  
(`static_assert(std::is_same_v);`.)

For arguments, maybe more complicated.  I guess you could write a C++ class 
with conversion operators for every integer type, mark all of them except one 
"= delete", and pass it as an argument.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85118

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

[PATCH] D85236: [CUDA] Work around a bug in rint() caused by a broken implementation provided by CUDA.

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added a reviewer: jlebar.
Herald added subscribers: sanjoy.google, bixia, yaxunl.
Herald added a project: clang.
tra requested review of this revision.

Normally math functions are forwarded to __nv_* counterparts provided by CUDA's
libdevice bitcode. However, __nv_rint*() functions there have a bug -- they use
round() which rounds *up* instead of rounding towards the nearest integer, so we
end up with rint(2.5f) producing 3.0 instead of expected 2.0. The broken bitcode
is not actually used by NVCC itself, which has both a work-around in CUDA
headers and, in recent versions, uses correct implementations in NVCC's 
built-ins.

This patch implements equivalent workaround and directs rint/rintf to
__builtin_rint/rintf that produce correct results.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85236

Files:
  clang/lib/Headers/__clang_cuda_math.h


Index: clang/lib/Headers/__clang_cuda_math.h
===
--- clang/lib/Headers/__clang_cuda_math.h
+++ clang/lib/Headers/__clang_cuda_math.h
@@ -249,8 +249,9 @@
 __DEVICE__ float rhypotf(float __a, float __b) {
   return __nv_rhypotf(__a, __b);
 }
-__DEVICE__ double rint(double __a) { return __nv_rint(__a); }
-__DEVICE__ float rintf(float __a) { return __nv_rintf(__a); }
+// __nv_rint* in libdevice is buggy and produces incorrect results.
+__DEVICE__ double rint(double __a) { return __builtin_rint(__a); }
+__DEVICE__ float rintf(float __a) { return __builtin_rintf(__a); }
 __DEVICE__ double rnorm(int __a, const double *__b) {
   return __nv_rnorm(__a, __b);
 }


Index: clang/lib/Headers/__clang_cuda_math.h
===
--- clang/lib/Headers/__clang_cuda_math.h
+++ clang/lib/Headers/__clang_cuda_math.h
@@ -249,8 +249,9 @@
 __DEVICE__ float rhypotf(float __a, float __b) {
   return __nv_rhypotf(__a, __b);
 }
-__DEVICE__ double rint(double __a) { return __nv_rint(__a); }
-__DEVICE__ float rintf(float __a) { return __nv_rintf(__a); }
+// __nv_rint* in libdevice is buggy and produces incorrect results.
+__DEVICE__ double rint(double __a) { return __builtin_rint(__a); }
+__DEVICE__ float rintf(float __a) { return __builtin_rintf(__a); }
 __DEVICE__ double rnorm(int __a, const double *__b) {
   return __nv_rnorm(__a, __b);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Generally i think this is shaping up nicely, with the couple of minor nits 
addressed i'm happy to land this. Like, there are a few cornercases that we 
could address but we could do that in follow-up patches. Folks, do you see 
anything else here?




Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:131
+  }
+  return nullptr;
+}

You never ever check for this case. Therefore this function entirely boils down 
to `Call.getArgExpr(0)` which is shorter than `getFirstArgExpr(Call)` anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84600

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

[PATCH] D82675: [PowerPC] Implement Vector Extract Mask builtins in LLVM/Clang

[Baptiste Saleil via Phabricator via cfe-commits]

bsaleil accepted this revision as: bsaleil.
bsaleil 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/D82675/new/

https://reviews.llvm.org/D82675

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

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:110
+  SmartPtrChecker *Checker = Mgr.registerChecker();
+  NullDereferenceBugTypePtr = >NullDereferenceBugType;
 }

vrnithinkumar wrote:
> NoQ wrote:
> > Wait, i don't understand again. You're taking the bug type from that 
> > checker and using it in that checker. Why did you need to make it global? I 
> > thought your problem was about capturing the bug type from the //raw// null 
> > dereference checker?
> > Wait, i don't understand again. You're taking the bug type from that 
> > checker and using it in that checker. Why did you need to make it global? I 
> > thought your problem was about capturing the bug type from the //raw// null 
> > dereference checker?
> 
> The check for bug type is in `SmartPtrModeling` but the bug type is defined 
> in `SmartPtrChecker`
Aha, ok, i misunderstood again. So i guess we didn't need a new header then. 
That said, it's an interesting layering violation that we encounter in this 
design: it used to be up to the checker to attach a visitor and so should be 
activating a note tag, but note tags are part of modeling, not checking.

I guess that's just how it is and it's the responsibility of the checkers to 
inform the modeling about bug types. I guess the ultimate API could look like 
`BugReport->activateNoteTag(NoteTagTag TT)` (where `TT` is a crying smiley that 
cries about "tag tags" T_T). But that's a story for another time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84600

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

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:1997
   case ABIArgInfo::Ignore:
+  case ABIArgInfo::IndirectAliased:
 return false;

rjmccall wrote:
> arsenm wrote:
> > rjmccall wrote:
> > > In principle, this can be `inreg` just as much as Indirect can.
> > The IR verifier currently will reject byref + inreg
> Why?  `inreg` is essentially orthogonal.
Mostly inherited from the other similar attribute handling. It can be lifted if 
there's a use


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

https://reviews.llvm.org/D79744

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added a comment.

No problem! Thank you, John!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D85231: Protect against filenames with no extension at all.

[Sterling Augustine via Phabricator via cfe-commits]

saugustine created this revision.
saugustine added a reviewer: echristo.
Herald added a reviewer: JDevlieghere.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
saugustine requested review of this revision.

Such as the one in the darwin-dsymutil.c test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85231

Files:
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4675,6 +4675,8 @@
 std::string::size_type End = std::string::npos;
 if (!types::appendSuffixForType(JA.getType()))
   End = BaseName.rfind('.');
+if (End == StringRef::npos)
+  End = BaseName.size();
 SmallString<128> Suffixed(BaseName.substr(0, End));
 Suffixed += OffloadingPrefix;
 if (MultipleArchs && !BoundArch.empty()) {


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4675,6 +4675,8 @@
 std::string::size_type End = std::string::npos;
 if (!types::appendSuffixForType(JA.getType()))
   End = BaseName.rfind('.');
+if (End == StringRef::npos)
+  End = BaseName.size();
 SmallString<128> Suffixed(BaseName.substr(0, End));
 Suffixed += OffloadingPrefix;
 if (MultipleArchs && !BoundArch.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e18c6ef - [clang] improve diagnostics for misaligned and large atomics

[JF Bastien via cfe-commits]

Author: Thorsten Schuett
Date: 2020-08-04T11:10:29-07:00
New Revision: e18c6ef6b41a59af73bf5c3d7d52a8c53a471e5d

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

LOG: [clang] improve diagnostics for misaligned and large atomics

"Listing the alignment and access size (== expected alignment) in the warning
seems like a good idea."

solves PR 46947

  struct Foo {
struct Bar {
  void * a;
  void * b;
};
Bar bar;
  };

  struct ThirtyTwo {
struct Large {
  void * a;
  void * b;
  void * c;
  void * d;
};
Large bar;
  };

  void braz(Foo *foo, ThirtyTwo *braz) {
Foo::Bar bar;
__atomic_load(>bar, , __ATOMIC_RELAXED);

ThirtyTwo::Large foobar;
__atomic_load(>bar, , __ATOMIC_RELAXED);
  }

repro.cpp:21:3: warning: misaligned atomic operation may incur significant 
performance penalty; the expected (16 bytes) exceeds the actual alignment (8 
bytes) [-Watomic-alignment]
  __atomic_load(>bar, , __ATOMIC_RELAXED);
  ^
repro.cpp:24:3: warning: misaligned atomic operation may incur significant 
performance penalty; the expected (32 bytes) exceeds the actual alignment (8 
bytes) [-Watomic-alignment]
  __atomic_load(>bar, , __ATOMIC_RELAXED);
  ^
repro.cpp:24:3: warning: large atomic operation may incur significant 
performance penalty; the access size (32 bytes) exceeds the max lock-free size 
(16  bytes) [-Watomic-alignment]
3 warnings generated.

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticFrontendKinds.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/lib/CodeGen/CGAtomic.cpp
clang/test/CodeGen/atomics-sema-alignment.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td 
b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index b202d2abffa0..6434d92fd8fc 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -270,8 +270,16 @@ def err_ifunc_resolver_return : Error<
   "ifunc resolver function must return a pointer">;
 
 def warn_atomic_op_misaligned : Warning<
-  "%select{large|misaligned}0 atomic operation may incur "
-  "significant performance penalty">, InGroup>;
+  "misaligned atomic operation may incur "
+  "significant performance penalty"
+  "; the expected alignment (%0 bytes) exceeds the actual alignment (%1 
bytes)">,
+  InGroup;
+
+def warn_atomic_op_oversized : Warning<
+  "large atomic operation may incur "
+  "significant performance penalty"
+  "; the access size (%0 bytes) exceeds the max lock-free size (%1  bytes)">,
+InGroup;
 
 def warn_alias_with_section : Warning<
   "%select{alias|ifunc}1 will not be in section '%0' but in the same section "

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 1e829be4028e..be62461faef4 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -699,6 +699,7 @@ def ReorderInitList : DiagGroup<"reorder-init-list">;
 def Reorder : DiagGroup<"reorder", [ReorderCtor, ReorderInitList]>;
 def UndeclaredSelector : DiagGroup<"undeclared-selector">;
 def ImplicitAtomic : DiagGroup<"implicit-atomic-properties">;
+def AtomicAlignment : DiagGroup<"atomic-alignment">;
 def CustomAtomic : DiagGroup<"custom-atomic-properties">;
 def AtomicProperties : DiagGroup<"atomic-properties",
  [ImplicitAtomic, CustomAtomic]>;

diff  --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index a58450ddd4c5..b7ada4ac7e3b 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -807,10 +807,20 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   bool Oversized = getContext().toBits(sizeChars) > MaxInlineWidthInBits;
   bool Misaligned = (Ptr.getAlignment() % sizeChars) != 0;
   bool UseLibcall = Misaligned | Oversized;
+  CharUnits MaxInlineWidth =
+  getContext().toCharUnitsFromBits(MaxInlineWidthInBits);
 
-  if (UseLibcall) {
-CGM.getDiags().Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
-<< !Oversized;
+  DiagnosticsEngine  = CGM.getDiags();
+
+  if (Misaligned) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
+<< (int)sizeChars.getQuantity()
+<< (int)Ptr.getAlignment().getQuantity();
+  }
+
+  if (Oversized) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_oversized)
+<< (int)sizeChars.getQuantity() << (int)MaxInlineWidth.getQuantity();
   }
 
   llvm::Value *Order = EmitScalarExpr(E->getOrder());

diff  --git a/clang/test/CodeGen/atomics-sema-alignment.c 
b/clang/test/CodeGen/atomics-sema-alignment.c
index 

[PATCH] D85102: [clang] improve diagnostics for misaligned and large atomics

[JF Bastien via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe18c6ef6b41a: [clang] improve diagnostics for misaligned and 
large atomics (authored by tschuett, committed by jfb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85102

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CGAtomic.cpp
  clang/test/CodeGen/atomics-sema-alignment.c

Index: clang/test/CodeGen/atomics-sema-alignment.c
===
--- clang/test/CodeGen/atomics-sema-alignment.c
+++ clang/test/CodeGen/atomics-sema-alignment.c
@@ -12,10 +12,10 @@
 
 void func(IntPair *p) {
   IntPair res;
-  __atomic_load(p, , 0); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
-  __atomic_store(p, , 0); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
-  __atomic_fetch_add((unaligned_int *)p, 1, 2); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
-  __atomic_fetch_sub((unaligned_int *)p, 1, 3); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
+  __atomic_load(p, , 0);// expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (8 bytes) exceeds the actual alignment (4 bytes)}}
+  __atomic_store(p, , 0);   // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (8 bytes) exceeds the actual alignment (4 bytes)}}
+  __atomic_fetch_add((unaligned_int *)p, 1, 2); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (4 bytes) exceeds the actual alignment (1 bytes)}}
+  __atomic_fetch_sub((unaligned_int *)p, 1, 3); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (4 bytes) exceeds the actual alignment (1 bytes)}}
 }
 
 void func1(LongStruct *p) {
@@ -25,3 +25,24 @@
   __atomic_fetch_add((int *)p, 1, 2);
   __atomic_fetch_sub((int *)p, 1, 3);
 }
+
+typedef struct {
+  void *a;
+  void *b;
+} Foo;
+
+typedef struct {
+  void *a;
+  void *b;
+  void *c;
+  void *d;
+} __attribute__((aligned(32))) ThirtyTwo;
+
+void braz(Foo *foo, ThirtyTwo *braz) {
+  Foo bar;
+  __atomic_load(foo, , __ATOMIC_RELAXED); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (16 bytes) exceeds the actual alignment (8 bytes)}}
+
+  ThirtyTwo thirtyTwo1;
+  ThirtyTwo thirtyTwo2;
+  __atomic_load(, , __ATOMIC_RELAXED); // expected-warning {{large atomic operation may incur significant performance penalty; the access size (32 bytes) exceeds the max lock-free size (16  bytes)}}
+}
Index: clang/lib/CodeGen/CGAtomic.cpp
===
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -807,10 +807,20 @@
   bool Oversized = getContext().toBits(sizeChars) > MaxInlineWidthInBits;
   bool Misaligned = (Ptr.getAlignment() % sizeChars) != 0;
   bool UseLibcall = Misaligned | Oversized;
+  CharUnits MaxInlineWidth =
+  getContext().toCharUnitsFromBits(MaxInlineWidthInBits);
 
-  if (UseLibcall) {
-CGM.getDiags().Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
-<< !Oversized;
+  DiagnosticsEngine  = CGM.getDiags();
+
+  if (Misaligned) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
+<< (int)sizeChars.getQuantity()
+<< (int)Ptr.getAlignment().getQuantity();
+  }
+
+  if (Oversized) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_oversized)
+<< (int)sizeChars.getQuantity() << (int)MaxInlineWidth.getQuantity();
   }
 
   llvm::Value *Order = EmitScalarExpr(E->getOrder());
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -699,6 +699,7 @@
 def Reorder : DiagGroup<"reorder", [ReorderCtor, ReorderInitList]>;
 def UndeclaredSelector : DiagGroup<"undeclared-selector">;
 def ImplicitAtomic : DiagGroup<"implicit-atomic-properties">;
+def AtomicAlignment : DiagGroup<"atomic-alignment">;
 def CustomAtomic : DiagGroup<"custom-atomic-properties">;
 def AtomicProperties : DiagGroup<"atomic-properties",
  [ImplicitAtomic, CustomAtomic]>;
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -270,8 

[PATCH] D83242: [clang][BPF] support type exist/size and enum exist/value relocations

[Andrii Nakryiko via Phabricator via cfe-commits]

anakryiko added a comment.

> This is different than testing whether a type exists or not where we want to 
> precisely test whether that type exists or not, so we want to preserve 
> `typedef` type vs. `struct t` type.

`typedef t` and `struct t` are also different for field relocations (those t is 
in different namespaces, actually), so it might be a good idea to disambiguate 
those for field relocations as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83242

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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added a comment.

What is expected to happen to device statics in anonymous name spaces? It may 
be worth adding them to the tests.

LGTM otherwise.


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

https://reviews.llvm.org/D80858

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Sorry, this slipped out of my mind.  I've started the process internally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> I guess with what you're suggesting the bitcast could still be emitted there 
> but the cast operations could be limited in Sema to cases where ultimately 
> ConvertType would return a type that requires bitcasting, or are you saying 
> that could be avoided completely?

The bitcast operation would exist in Sema either way; it's necessary for the 
types to stay consistent.  My suggestion is just that bitcasting between a VLAT 
and the corresponding VLST it would be a no-op in CodeGen.

My suggestion is similar to the way the "bool" type works: in memory, it's an 
i8, but when you load it, it's truncated it to i1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85128

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

[PATCH] D84457: [OPENMP]Fix PR37671: Privatize local(private) variables in untied tasks.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 282958.
ABataev added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84457

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/task_codegen.cpp

Index: clang/test/OpenMP/task_codegen.cpp
===
--- clang/test/OpenMP/task_codegen.cpp
+++ clang/test/OpenMP/task_codegen.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix UNTIEDRT
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 //
@@ -258,7 +258,7 @@
 a = 4;
 c = 5;
   }
-// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i32 0, i64 40, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
+// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i32 0, i64 48, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
 // CHECK: call i32 @__kmpc_omp_task([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i8* [[ORIG_TASK_PTR]])
 #pragma omp task untied
   {
@@ -295,26 +295,54 @@
 // CHECK: store i32 4, i32* [[A_PTR]]
 
 // CHECK: define internal i32 [[TASK_ENTRY6]](i32 %0, [[KMP_TASK_T]]{{.*}}* noalias %1)
-// CHECK: switch i32 %{{.+}}, label
+// UNTIEDRT: [[S1_ADDR_PTR:%.+]] = alloca %struct.S*,
+// UNTIEDRT: call void (i8*, ...) %{{.+}}(i8* %{{.+}}, %struct.S** [[S1_ADDR_PTR]])
+// UNTIEDRT: [[S1_ADDR:%.+]] = load %struct.S*, %struct.S** [[S1_ADDR_PTR]],
+// CHECK: switch i32 %{{.+}}, label %[[DONE:.+]] [
+
+// CHECK: [[DONE]]:
+// CHECK: br label %[[CLEANUP:[^,]+]]
+
 // CHECK: load i32*, i32** %
 // CHECK: store i32 1, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT:[^,]+]]
 
+// UNTIEDRT: call void [[CONSTR:@.+]](%struct.S* [[S1_ADDR]])
 // CHECK: call i8* @__kmpc_omp_task_alloc(
 // CHECK: call i32 @__kmpc_omp_task(%
 // CHECK: load i32*, i32** %
 // CHECK: store i32 2, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT]]
 
 // CHECK: call i32 @__kmpc_omp_taskyield(%
 // CHECK: load i32*, i32** %
 // CHECK: store i32 3, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT]]
+
+// s1 = S();
+// UNTIEDRT: call void [[CONSTR]](%struct.S* [[TMP:%.+]])
+// UNTIEDRT: [[DST:%.+]] = bitcast %struct.S* [[S1_ADDR]] to i8*
+// UNTIEDRT: [[SRC:%.+]] = bitcast %struct.S* [[TMP]] to i8*
+// UNTIEDRT: call void @llvm.memcpy.{{.+}}(i8* {{.*}}[[DST]], i8* {{.*}}[[SRC]], i64 4, i1 false)
+// UNTIEDRT: call void [[DESTR:@.+]](%struct.S* [[TMP]])
 
 // CHECK: call i32 @__kmpc_omp_taskwait(%
 // CHECK: load i32*, i32** %
 // CHECK: store i32 4, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT]]
+
+// UNTIEDRT: call void [[DESTR]](%struct.S* [[S1_ADDR]])
+// CHECK: br label %[[CLEANUP]]
+
+// CHECK: [[CLEANUP]]:
+// UNTIEDRT: br label %[[EXIT]]
+
+// UNTIEDRT:  [[EXIT]]:
+// UNTIEDRT-NEXT: ret i32 0
 
 struct S1 {
   int a;
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/OpenMPClause.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/PrettyStackTrace.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
@@ -3787,6 +3788,42 @@
   checkForLastprivateConditionalUpdate(*this, S);
 }
 
+namespace {
+/// Get the list of variables declared in the context of the untied tasks.
+class CheckVarsEscapingUntiedTaskDeclContext final
+: public ConstStmtVisitor {
+  llvm::SmallVector PrivateDecls;
+
+public:
+  explicit CheckVarsEscapingUntiedTaskDeclContext() = default;
+  virtual ~CheckVarsEscapingUntiedTaskDeclContext() = default;
+  void VisitDeclStmt(const DeclStmt *S) {
+if (!S)
+  return;
+// Need to privatize only local vars, static locals can be processed as is.
+for (const Decl *D : S->decls()) {
+  if (const auto *VD = dyn_cast_or_null(D))
+if (VD->hasLocalStorage())
+  PrivateDecls.push_back(VD);
+}
+  }
+  void VisitOMPExecutableDirective(const OMPExecutableDirective *) { return; }
+  void VisitCapturedStmt(const CapturedStmt *) { return; }
+  void VisitLambdaExpr(const LambdaExpr *) { return; }

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll planned changes to this revision.
simoll added a comment.

Thanks for the feedback!
Planned change: interpret the `N` in `vector_size(N)` as the number of bytes in 
the type, such that, for example, a `vector_size(2)` bool vector holds 16 bits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[clang] 83cb98f - Fix sphinx indentation warnings by adding explicit line breaks to address space hierarchy

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-08-04T17:48:54+01:00
New Revision: 83cb98f9e7a57360e137b32b26500fca630df617

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

LOG: Fix sphinx indentation warnings by adding explicit line breaks to address 
space hierarchy

Added: 


Modified: 
clang/include/clang/Basic/AttrDocs.td

Removed: 




diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 76a075a97ee1..83990721d7f7 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3134,11 +3134,12 @@ distinguish USM (Unified Shared Memory) pointers that 
access global device
 memory from those that access global host memory. These new address spaces are
 a subset of the ``__global/opencl_global`` address space, the full address 
space
 set model for OpenCL 2.0 with the extension looks as follows:
-  generic->global->host
- ->device
- ->private
- ->local
-  constant
+
+  | generic->global->host
+  |->device
+  |->private
+  |->local
+  | constant
 
 As ``global_device`` and ``global_host`` are a subset of
 ``__global/opencl_global`` address spaces it is allowed to convert



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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

simoll wrote:
> rsandifo-arm wrote:
> > simoll wrote:
> > > rsandifo-arm wrote:
> > > > simoll wrote:
> > > > > rsandifo-arm wrote:
> > > > > > simoll wrote:
> > > > > > > lenary wrote:
> > > > > > > > It would be nice if this aside about non-bool vectors was more 
> > > > > > > > prominently displayed - it's something I hadn't realised before.
> > > > > > > Yep. that caught me by surprise too. I'll move that sentence to 
> > > > > > > the paragraph about GCC vectors above.
> > > > > > Sorry for the extremely late comment.  Like @lenary I hadn't 
> > > > > > thought about this.  I'd assumed that the vector woiuld still be a 
> > > > > > multiple of 8 bits in size, but I agree that's probably too 
> > > > > > restrictive to be the only option available.
> > > > > > 
> > > > > > In that case, would it make sense to add a separate attribute 
> > > > > > instead?  I think it's too surprising to change the units of the 
> > > > > > existing attribute based on the element type.  Perhaps we should 
> > > > > > even make it take two parameters: the total number of elements, and 
> > > > > > the number of bits per element.  That might be more natural for 
> > > > > > some AVX and SVE combinations.  We wouldn't need to supporrt all 
> > > > > > combinations from the outset, it's just a question whether we 
> > > > > > should make the syntax general enough to support it in future.
> > > > > > 
> > > > > > Perhaps we could do both: support `vector_size` for `bool` using 
> > > > > > byte sizes (and not allowing subbyte vector lengths), and add a 
> > > > > > new, more general attribute that allows subbyte lengths and 
> > > > > > explicit subbyte element sizes.
> > > > > > In that case, would it make sense to add a separate attribute 
> > > > > > instead? I think it's too surprising to change the units of the 
> > > > > > existing attribute based on the element type. Perhaps we should 
> > > > > > even make it take two parameters: the total number of elements, and 
> > > > > > the number of bits per element. That might be more natural for some 
> > > > > > AVX and SVE combinations. We wouldn't need to supporrt all 
> > > > > > combinations from the outset, it's just a question whether we 
> > > > > > should make the syntax general enough to support it in future.
> > > > > 
> > > > > I guess adding a new attribute makes sense mid to long term. For now, 
> > > > > i'd want something that just does the job... ie, what is proposed 
> > > > > here. We should absolutely document the semantics of vector_size 
> > > > > properly.. it already is counterintuitive (broken, if you ask me).
> > > > > 
> > > > > 
> > > > > > Perhaps we could do both: support vector_size for bool using byte 
> > > > > > sizes (and not allowing subbyte vector lengths), and add a new, 
> > > > > > more general attribute that allows subbyte lengths and explicit 
> > > > > > subbyte element sizes.
> > > > > 
> > > > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > > > because these types are produced implicitly by compares of (legal) 
> > > > > vector types. Consider this:
> > > > > 
> > > > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > > > >   int3 A = ...;
> > > > >   int3 B = ...;
> > > > >   auto Z = A < B; // vector compare yielding a `bool 
> > > > > vector_size(3)`-typed value.
> > > > > 
> > > > > 
> > > > > Regarding your proposal for a separate subbyte element size and 
> > > > > subbyte length, that may or may not make sense but it is surely 
> > > > > something that should be discussed more broadly with its own proposal.
> > > > > > Perhaps we could do both: support vector_size for bool using byte 
> > > > > > sizes (and not allowing subbyte vector lengths), and add a new, 
> > > > > > more general attribute that allows subbyte lengths and explicit 
> > > > > > subbyte element sizes.
> > > > > 
> > > > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > > > because these types are produced implicitly by compares of (legal) 
> > > > > vector types. Consider this:
> > > > > 
> > > > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > > > >   int3 A = ...;
> > > > >   int3 B = ...;
> > > > >   auto Z = A < B; // vector compare yielding a `bool 
> > > > > vector_size(3)`-typed value.
> > > > 
> > > > Yeah, I understand the need for some way of creating subbyte vectors.  
> > > > I'm just not sure using the existing `vector_size` attribute would be 
> > > > the best choice for that case.  I.e. the objection wasn't based on 
> > > > “keeping things simple” but more “keeping things consistent“.
> > > > 
> > > > That doesn't mean that the above 

[PATCH] D85223: [CUDA][HIP] Support accessing static device variable in host code for -fgpu-rdc

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall, JonChesterfield, hliao.
Herald added a subscriber: dang.
yaxunl requested review of this revision.

This is separated from https://reviews.llvm.org/D80858

For -fgpu-rdc mode, static device vars in different TU's may have the same name.
To support accessing file-scope static device variables in host code, we need 
to give them
a distinct name and external linkage. This can be done by postfixing each 
static device variable with
a distinct CUID (Compilation Unit ID). Also we have to make sure the host 
compilation and device
compilation of the same compilation unit use identical CUID.

This patch added a distinct CUID for each input file, which is represented by 
InputAction.
clang initially creates an InputAction for each input file for the host 
compilation. In CUDA/HIP action
builder, each InputAction is given a CUID and cloned for each GPU arch, and the 
CUID is also cloned. In this way,
we guarantee the corresponding device and host compilation for the same file 
shared the
same CUID, therefore the postfixed device variable and shadow variable share 
the same name.
On the other hand, different compilation units have different CUID, therefore a 
static variable
with the same name but in a different compilation unit will have a different 
name.

Since the static device variables have different name across compilation units, 
now we let
them have external linkage so that they can be looked up by the runtime.

-fuse-cuid=random|hash|none is added to control the method to generate CUID. 
The default
is hash. -cuid=X is also added to specify CUID explicitly, which overrides 
-fuse-cuid.


https://reviews.llvm.org/D85223

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Action.h
  clang/include/clang/Driver/Compilation.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Driver/Action.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCUDA/static-device-var-rdc.cu
  clang/test/Driver/hip-cuid.hip
  clang/test/Frontend/hip-cuid.hip
  clang/test/SemaCUDA/static-device-var.cu

Index: clang/test/SemaCUDA/static-device-var.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/static-device-var.cu
@@ -0,0 +1,37 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple nvptx -fcuda-is-device \
+// RUN:-emit-llvm -o - %s -fsyntax-only -verify
+
+// RUN: %clang_cc1 -triple x86_64-gnu-linux \
+// RUN:-emit-llvm -o - %s -fsyntax-only -verify
+
+#include "Inputs/cuda.h"
+
+__device__ void f1() {
+  const static int b = 123;
+  static int a;
+  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+}
+
+__global__ void k1() {
+  const static int b = 123;
+  static int a;
+  // expected-error@-1 {{within a __global__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+}
+
+static __device__ int x;
+static __constant__ int y;
+
+__global__ void kernel(int *a) {
+  a[0] = x;
+  a[1] = y;
+}
+
+int* getDeviceSymbol(int *x);
+
+void foo() {
+  getDeviceSymbol();
+  getDeviceSymbol();
+}
Index: clang/test/Frontend/hip-cuid.hip
===
--- /dev/null
+++ clang/test/Frontend/hip-cuid.hip
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -cuid=abc-123 -offload-arch=gfx906 %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=INVALID %s
+
+// INVALID: invalid value 'abc-123' in '-cuid=abc-123' (alphanumeric characters and underscore only)
+
+// RUN: %clang_cc1 -cuid=abc_123 -offload-arch=gfx906 %s
Index: clang/test/Driver/hip-cuid.hip
===
--- /dev/null
+++ clang/test/Driver/hip-cuid.hip
@@ -0,0 +1,130 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// Check invalid -fuse-cuid= option.
+
+// RUN: not %clang -### -x hip \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   --offload-arch=gfx906 \
+// RUN:   -c -nogpulib -fuse-cuid=invalid \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck -check-prefixes=INVALID %s
+
+// INVALID: invalid value 'invalid' in '-fuse-cuid=invalid'
+
+// Check random CUID generator.
+
+// RUN: %clang -### -x hip \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   --offload-arch=gfx906 \
+// RUN:   -c -nogpulib 

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 282952.
yaxunl retitled this revision from "[CUDA][HIP] Support accessing static device 
variable in host code" to "[CUDA][HIP] Support accessing static device variable 
in host code for -fno-gpu-rdc".
yaxunl edited the summary of this revision.
yaxunl added a comment.

revised for -fno-gpu-rdc case by Michael's comments.


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

https://reviews.llvm.org/D80858

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu

Index: clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
@@ -0,0 +1,86 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN:   -emit-llvm -o - -x hip %s | FileCheck \
+// RUN:   -check-prefixes=DEV %s
+
+// RUN: %clang_cc1 -triple x86_64-gnu-linux \
+// RUN:   -emit-llvm -o - -x hip %s | FileCheck \
+// RUN:   -check-prefixes=HOST %s
+
+#include "Inputs/cuda.h"
+
+// Test function scope static device variable, which should not be externalized.
+// DEV-DAG: @_ZZ6kernelPiPPKiE1w = internal addrspace(4) constant i32 1
+
+// Check a static device variable referenced by host function is externalized.
+// DEV-DAG: @_ZL1x = addrspace(1) externally_initialized global i32 0
+// HOST-DAG: @_ZL1x = internal global i32 undef
+// HOST-DAG: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x\00"
+
+static __device__ int x;
+
+// Check a static device variables referenced only by device functions and kernels
+// is not externalized.
+// DEV-DAG: @_ZL2x2 = internal addrspace(1) global i32 0
+static __device__ int x2;
+
+// Check a static device variable referenced by host device function is externalized.
+// DEV-DAG: @_ZL2x3 = addrspace(1) externally_initialized global i32 0
+static __device__ int x3;
+
+// Check a static device variable referenced in file scope is externalized.
+// DEV-DAG: @_ZL2x4 = addrspace(1) externally_initialized global i32 0
+static __device__ int x4;
+int& x4_ref = x4;
+
+// Check a static constant variable referenced by host is externalized.
+// DEV-DAG: @_ZL1y = addrspace(4) externally_initialized global i32 0
+// HOST-DAG: @_ZL1y = internal global i32 undef
+// HOST-DAG: @[[DEVNAMEY:[0-9]+]] = {{.*}}c"_ZL1y\00"
+
+static __constant__ int y;
+
+// Test static host variable, which should not be externalized nor registered.
+// HOST-DAG: @_ZL1z = internal global i32 0
+// DEV-NOT: @_ZL1z
+static int z;
+
+// Test static device variable in inline function, which should not be
+// externalized nor registered.
+// DEV-DAG: @_ZZ6devfunPPKiE1p = linkonce_odr addrspace(4) constant i32 2, comdat
+
+inline __device__ void devfun(const int ** b) {
+  const static int p = 2;
+  b[0] = 
+  b[1] = 
+}
+
+__global__ void kernel(int *a, const int **b) {
+  const static int w = 1;
+  a[0] = x;
+  a[1] = y;
+  a[2] = x2;
+  a[3] = x3;
+  a[4] = x4;
+  b[0] = 
+  devfun(b);
+}
+
+__host__ __device__ void hdf(int *a) {
+  a[0] = x3;
+}
+
+int* getDeviceSymbol(int *x);
+
+void foo(int *a) {
+  getDeviceSymbol();
+  getDeviceSymbol();
+  z = 123;
+}
+
+// HOST: __hipRegisterVar({{.*}}@_ZL1x {{.*}}@[[DEVNAMEX]]
+// HOST: __hipRegisterVar({{.*}}@_ZL1y {{.*}}@[[DEVNAMEY]]
+// HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6kernelPiPPKiE1w
+// HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6devfunPPKiE1p
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17873,6 +17873,25 @@
   if (Var->isInvalidDecl())
 return;
 
+  // Record a CUDA/HIP static device/constant variable if it is referenced
+  // by host code. This is done conservatively, when the variable is referenced
+  // in any of the following contexts:
+  //   - a non-function context
+  //   - a host function
+  //   - a host device function
+  // This also requires the reference of the static device/constant variable by
+  // host code to be visible in the device compilation for the compiler to be
+  // able to externalize the static device/constant variable.
+  if ((Var->hasAttr() || Var->hasAttr()) &&
+  Var->isFileVarDecl() && Var->getStorageClass() == SC_Static) {
+auto *CurContext = SemaRef.CurContext;
+if (!CurContext || !isa(CurContext) ||
+cast(CurContext)->hasAttr() ||
+(!cast(CurContext)->hasAttr() &&
+ !cast(CurContext)->hasAttr()))
+  SemaRef.getASTContext().CUDAStaticDeviceVarReferencedByHost.insert(Var);
+  }
+
   auto *MSI = Var->getMemberSpecializationInfo();
   TemplateSpecializationKind TSK = MSI ? MSI->getTemplateSpecializationKind()
: Var->getTemplateSpecializationKind();
Index: clang/lib/AST/ASTContext.cpp

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

rsandifo-arm wrote:
> simoll wrote:
> > rsandifo-arm wrote:
> > > simoll wrote:
> > > > rsandifo-arm wrote:
> > > > > simoll wrote:
> > > > > > lenary wrote:
> > > > > > > It would be nice if this aside about non-bool vectors was more 
> > > > > > > prominently displayed - it's something I hadn't realised before.
> > > > > > Yep. that caught me by surprise too. I'll move that sentence to the 
> > > > > > paragraph about GCC vectors above.
> > > > > Sorry for the extremely late comment.  Like @lenary I hadn't thought 
> > > > > about this.  I'd assumed that the vector woiuld still be a multiple 
> > > > > of 8 bits in size, but I agree that's probably too restrictive to be 
> > > > > the only option available.
> > > > > 
> > > > > In that case, would it make sense to add a separate attribute 
> > > > > instead?  I think it's too surprising to change the units of the 
> > > > > existing attribute based on the element type.  Perhaps we should even 
> > > > > make it take two parameters: the total number of elements, and the 
> > > > > number of bits per element.  That might be more natural for some AVX 
> > > > > and SVE combinations.  We wouldn't need to supporrt all combinations 
> > > > > from the outset, it's just a question whether we should make the 
> > > > > syntax general enough to support it in future.
> > > > > 
> > > > > Perhaps we could do both: support `vector_size` for `bool` using byte 
> > > > > sizes (and not allowing subbyte vector lengths), and add a new, more 
> > > > > general attribute that allows subbyte lengths and explicit subbyte 
> > > > > element sizes.
> > > > > In that case, would it make sense to add a separate attribute 
> > > > > instead? I think it's too surprising to change the units of the 
> > > > > existing attribute based on the element type. Perhaps we should even 
> > > > > make it take two parameters: the total number of elements, and the 
> > > > > number of bits per element. That might be more natural for some AVX 
> > > > > and SVE combinations. We wouldn't need to supporrt all combinations 
> > > > > from the outset, it's just a question whether we should make the 
> > > > > syntax general enough to support it in future.
> > > > 
> > > > I guess adding a new attribute makes sense mid to long term. For now, 
> > > > i'd want something that just does the job... ie, what is proposed here. 
> > > > We should absolutely document the semantics of vector_size properly.. 
> > > > it already is counterintuitive (broken, if you ask me).
> > > > 
> > > > 
> > > > > Perhaps we could do both: support vector_size for bool using byte 
> > > > > sizes (and not allowing subbyte vector lengths), and add a new, more 
> > > > > general attribute that allows subbyte lengths and explicit subbyte 
> > > > > element sizes.
> > > > 
> > > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > > because these types are produced implicitly by compares of (legal) 
> > > > vector types. Consider this:
> > > > 
> > > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > > >   int3 A = ...;
> > > >   int3 B = ...;
> > > >   auto Z = A < B; // vector compare yielding a `bool 
> > > > vector_size(3)`-typed value.
> > > > 
> > > > 
> > > > Regarding your proposal for a separate subbyte element size and subbyte 
> > > > length, that may or may not make sense but it is surely something that 
> > > > should be discussed more broadly with its own proposal.
> > > > > Perhaps we could do both: support vector_size for bool using byte 
> > > > > sizes (and not allowing subbyte vector lengths), and add a new, more 
> > > > > general attribute that allows subbyte lengths and explicit subbyte 
> > > > > element sizes.
> > > > 
> > > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > > because these types are produced implicitly by compares of (legal) 
> > > > vector types. Consider this:
> > > > 
> > > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > > >   int3 A = ...;
> > > >   int3 B = ...;
> > > >   auto Z = A < B; // vector compare yielding a `bool 
> > > > vector_size(3)`-typed value.
> > > 
> > > Yeah, I understand the need for some way of creating subbyte vectors.  
> > > I'm just not sure using the existing `vector_size` attribute would be the 
> > > best choice for that case.  I.e. the objection wasn't based on “keeping 
> > > things simple” but more “keeping things consistent“.
> > > 
> > > That doesn't mean that the above code should be invalid.  It just means 
> > > that it wouldn't be possible to write the type of Z using the existing 
> > > `vector_size` attribute.
> > > 
> > > (FWIW, `vector_size` was 

[PATCH] D85218: In clang-tidy base checks prevent anonymous functions from triggering assertions

[Bogdan Serea via Phabricator via cfe-commits]

bogser01 created this revision.
bogser01 added reviewers: djasper, alexfh.
bogser01 added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
bogser01 requested review of this revision.

Skeleton checks generated by clang-tidy add_check.py cause assertions to fail 
when run over anonymous functions(lambda functions). This patch introduces an 
additional check to verify that the target function is not anonymous before 
calling getName(). 
The code snippet from the [[ 
https://clang.llvm.org/extra/clang-tidy/Contributing.html | clang-tidy tutorial 
 ]]is also updated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85218

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/docs/clang-tidy/Contributing.rst


Index: clang-tools-extra/docs/clang-tidy/Contributing.rst
===
--- clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -201,8 +201,8 @@
   }
 
   void AwesomeFunctionNamesCheck::check(const MatchFinder::MatchResult 
) {
-const auto *MatchedDecl = Result.Nodes.getNodeAs("x");
-if (MatchedDecl->getName().startswith("awesome_"))
+const auto *MatchedDecl = Result.Nodes.getNodeAs("x");
+if ((!MatchedDecl->getIdentifier()) || 
MatchedDecl->getName().startswith("awesome_"))
   return;
 diag(MatchedDecl->getLocation(), "function %0 is insufficiently awesome")
 << MatchedDecl
Index: clang-tools-extra/clang-tidy/add_new_check.py
===
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -136,7 +136,7 @@
 void %(check_name)s::check(const MatchFinder::MatchResult ) {
   // FIXME: Add callback implementation.
   const auto *MatchedDecl = Result.Nodes.getNodeAs("x");
-  if (MatchedDecl->getName().startswith("awesome_"))
+  if ((!MatchedDecl->getIdentifier()) || 
MatchedDecl->getName().startswith("awesome_"))
 return;
   diag(MatchedDecl->getLocation(), "function %%0 is insufficiently awesome")
   << MatchedDecl;


Index: clang-tools-extra/docs/clang-tidy/Contributing.rst
===
--- clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -201,8 +201,8 @@
   }
 
   void AwesomeFunctionNamesCheck::check(const MatchFinder::MatchResult ) {
-const auto *MatchedDecl = Result.Nodes.getNodeAs("x");
-if (MatchedDecl->getName().startswith("awesome_"))
+const auto *MatchedDecl = Result.Nodes.getNodeAs("x");
+if ((!MatchedDecl->getIdentifier()) || MatchedDecl->getName().startswith("awesome_"))
   return;
 diag(MatchedDecl->getLocation(), "function %0 is insufficiently awesome")
 << MatchedDecl
Index: clang-tools-extra/clang-tidy/add_new_check.py
===
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -136,7 +136,7 @@
 void %(check_name)s::check(const MatchFinder::MatchResult ) {
   // FIXME: Add callback implementation.
   const auto *MatchedDecl = Result.Nodes.getNodeAs("x");
-  if (MatchedDecl->getName().startswith("awesome_"))
+  if ((!MatchedDecl->getIdentifier()) || MatchedDecl->getName().startswith("awesome_"))
 return;
   diag(MatchedDecl->getLocation(), "function %%0 is insufficiently awesome")
   << MatchedDecl;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

[Nithin VR via Phabricator via cfe-commits]

vrnithinkumar added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:110
+  SmartPtrChecker *Checker = Mgr.registerChecker();
+  NullDereferenceBugTypePtr = >NullDereferenceBugType;
 }

NoQ wrote:
> Wait, i don't understand again. You're taking the bug type from that checker 
> and using it in that checker. Why did you need to make it global? I thought 
> your problem was about capturing the bug type from the //raw// null 
> dereference checker?
> Wait, i don't understand again. You're taking the bug type from that checker 
> and using it in that checker. Why did you need to make it global? I thought 
> your problem was about capturing the bug type from the //raw// null 
> dereference checker?

The check for bug type is in `SmartPtrModeling` but the bug type is defined in 
`SmartPtrChecker`



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:95-96
+
+  const auto *RecordDecl = MethodDecl->getParent();
+  if (RecordDecl && RecordDecl->getDeclName().isIdentifier()) {
+OS << " of type '" << RecordDecl->getQualifiedNameAsString() << "'";

NoQ wrote:
> vrnithinkumar wrote:
> > NoQ wrote:
> > > Aha, this time you checked for anonymous declarations! Good.
> > > 
> > > That said, i'm not sure type is actually useful for this warning, because 
> > > they're roughly all the same.
> > Okay. I was trying to be consistent with `MoveChecker`.
> > Just removed smart pointer type.
> These were important in `MoveChecker` because use-after-move rules for smart 
> pointers are different from use-after-move rules of any other class in the 
> standard library and the checker has to behave differently 
> (https://wiki.sei.cmu.edu/confluence/x/O3s-BQ).
Oh
That makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84600

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

simoll wrote:
> rsandifo-arm wrote:
> > simoll wrote:
> > > rsandifo-arm wrote:
> > > > simoll wrote:
> > > > > lenary wrote:
> > > > > > It would be nice if this aside about non-bool vectors was more 
> > > > > > prominently displayed - it's something I hadn't realised before.
> > > > > Yep. that caught me by surprise too. I'll move that sentence to the 
> > > > > paragraph about GCC vectors above.
> > > > Sorry for the extremely late comment.  Like @lenary I hadn't thought 
> > > > about this.  I'd assumed that the vector woiuld still be a multiple of 
> > > > 8 bits in size, but I agree that's probably too restrictive to be the 
> > > > only option available.
> > > > 
> > > > In that case, would it make sense to add a separate attribute instead?  
> > > > I think it's too surprising to change the units of the existing 
> > > > attribute based on the element type.  Perhaps we should even make it 
> > > > take two parameters: the total number of elements, and the number of 
> > > > bits per element.  That might be more natural for some AVX and SVE 
> > > > combinations.  We wouldn't need to supporrt all combinations from the 
> > > > outset, it's just a question whether we should make the syntax general 
> > > > enough to support it in future.
> > > > 
> > > > Perhaps we could do both: support `vector_size` for `bool` using byte 
> > > > sizes (and not allowing subbyte vector lengths), and add a new, more 
> > > > general attribute that allows subbyte lengths and explicit subbyte 
> > > > element sizes.
> > > > In that case, would it make sense to add a separate attribute instead? 
> > > > I think it's too surprising to change the units of the existing 
> > > > attribute based on the element type. Perhaps we should even make it 
> > > > take two parameters: the total number of elements, and the number of 
> > > > bits per element. That might be more natural for some AVX and SVE 
> > > > combinations. We wouldn't need to supporrt all combinations from the 
> > > > outset, it's just a question whether we should make the syntax general 
> > > > enough to support it in future.
> > > 
> > > I guess adding a new attribute makes sense mid to long term. For now, i'd 
> > > want something that just does the job... ie, what is proposed here. We 
> > > should absolutely document the semantics of vector_size properly.. it 
> > > already is counterintuitive (broken, if you ask me).
> > > 
> > > 
> > > > Perhaps we could do both: support vector_size for bool using byte sizes 
> > > > (and not allowing subbyte vector lengths), and add a new, more general 
> > > > attribute that allows subbyte lengths and explicit subbyte element 
> > > > sizes.
> > > 
> > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > because these types are produced implicitly by compares of (legal) vector 
> > > types. Consider this:
> > > 
> > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > >   int3 A = ...;
> > >   int3 B = ...;
> > >   auto Z = A < B; // vector compare yielding a `bool 
> > > vector_size(3)`-typed value.
> > > 
> > > 
> > > Regarding your proposal for a separate subbyte element size and subbyte 
> > > length, that may or may not make sense but it is surely something that 
> > > should be discussed more broadly with its own proposal.
> > > > Perhaps we could do both: support vector_size for bool using byte sizes 
> > > > (and not allowing subbyte vector lengths), and add a new, more general 
> > > > attribute that allows subbyte lengths and explicit subbyte element 
> > > > sizes.
> > > 
> > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > because these types are produced implicitly by compares of (legal) vector 
> > > types. Consider this:
> > > 
> > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > >   int3 A = ...;
> > >   int3 B = ...;
> > >   auto Z = A < B; // vector compare yielding a `bool 
> > > vector_size(3)`-typed value.
> > 
> > Yeah, I understand the need for some way of creating subbyte vectors.  I'm 
> > just not sure using the existing `vector_size` attribute would be the best 
> > choice for that case.  I.e. the objection wasn't based on “keeping things 
> > simple” but more “keeping things consistent“.
> > 
> > That doesn't mean that the above code should be invalid.  It just means 
> > that it wouldn't be possible to write the type of Z using the existing 
> > `vector_size` attribute.
> > 
> > (FWIW, `vector_size` was originally a GNUism and GCC stil requires vector 
> > sizes to be a power of 2, but I realise that isn't relevant here.  And the 
> > same principle applies with s/3/4 in the above example 

[PATCH] D84534: [AIX] Static init frontend recovery and backend support

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 6 inline comments as done.
Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.h:1058
+  /// Add an sterm finalizer to its own llvm.global_dtors entry.
+  void AddCXXStermFinalizerToGlobalDtor(llvm::Function *StermFinalizer,
+int Priority) {

jasonliu wrote:
> This wrapper seems redundant. Calling ` AddGlobalDtor(StermFinalizer, 
> Priority);` directly from the callee side already convey what we are trying 
> to achieve here. 
I added this wrapper function to keep the symmetry between `AddGlobalCtor` and 
`AddGlobalDtor`. They are private functions within `CodeGenModule` class. And I 
feel it's kinda weird to only move `AddGlobalDtor` to a public function.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4602
+llvm::report_fatal_error(
+"prioritized __sinit and __sterm functions are not yet supported");
+  else if (isTemplateInstantiation(D.getTemplateSpecializationKind()) ||

jasonliu wrote:
> Could we trigger this error? If so, could we have a test for it? 
I should've put an assertion here. The init priority attribute has been 
disabled on AIX in the previous patch.



Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1874
+  }
+  emitSpecialLLVMGlobal();
+  continue;

jasonliu wrote:
> Have some suggestion on structure backend code change:
> 
> 1. Remove `isSpecialLLVMGlobalArrayForStaticInit` and 
> `isSpecialLLVMGlobalArrayToSkip`, and call `emitSpecialLLVMGlobal` directly 
> instead. This would handle all the `llvm.` variable for us. We would need do 
> early return for names start with `llvm.` in emitGlobalVariable as well, 
> since they are all handled here.
> 
> 2. We could override emitXXStructorList because how XCOFF handle the list is 
> vastly different than the other target. We could make the common part(i.e. 
> processing and sorting) a utility function, say "preprocessStructorList".
> 
> 3. It's not necessary to use the same name `emitXXStructor` if the interface 
> is different. It might not provide much help when we kinda know no other 
> target is going to use this interface. So we could turn it into a lambda 
> inside of the overrided emitXXStructorList, or simply no need for a new 
> function because the logic is fairly straightforward.
> 
1. As we discussed offline, we would leave the handling of `llvm.used` and 
`llvm.metadata` later and don't include them in the scope of this patch.


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

https://reviews.llvm.org/D84534

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

[PATCH] D84887: [OPENMP]Fix codegen for is_device_ptr component, captured by reference.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 282942.
ABataev added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84887

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


Index: clang/test/OpenMP/target_is_device_ptr_codegen.cpp
===
--- clang/test/OpenMP/target_is_device_ptr_codegen.cpp
+++ clang/test/OpenMP/target_is_device_ptr_codegen.cpp
@@ -285,4 +285,42 @@
   ++arg;
 }
 #endif
+///==///
+// RUN: %clang_cc1 -DCK3 -verify -fopenmp 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple 
powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix 
CK3 --check-prefix CK3-64
+// RUN: %clang_cc1 -DCK3 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ 
-triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s 
-emit-llvm -o - | FileCheck %s  --check-prefix CK3 --check-prefix CK3-64
+// RUN: %clang_cc1 -DCK3 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu 
-x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s  
--check-prefix CK3 --check-prefix CK3-32
+// RUN: %clang_cc1 -DCK3 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ 
-std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple 
i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | 
FileCheck %s  --check-prefix CK3 --check-prefix CK3-32
+
+// RUN: %clang_cc1 -DCK3 -verify -fopenmp-simd 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple 
powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix 
SIMD-ONLY1 %s
+// RUN: %clang_cc1 -DCK3 -fopenmp-simd 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple 
powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x 
c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s 
-emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY1 %s
+// RUN: %clang_cc1 -DCK3 -verify -fopenmp-simd 
-fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown 
-emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY1 %s
+// RUN: %clang_cc1 -DCK3 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x 
c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ 
-triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm 
-o - | FileCheck --check-prefix SIMD-ONLY1 %s
+// SIMD-ONLY1-NOT: {{__kmpc|__tgt}}
+#ifdef CK3
+
+
+// CK3-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i[[SZ:64|32]]] [i{{64|32}} 
{{8|4}}]
+// OMP_MAP_TARGET_PARAM = 0x20 | OMP_MAP_TO = 0x1 = 0x21
+// CK3-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x21]]]
+void bar(){
+  __attribute__((aligned(64))) double *ptr;
+  // CK3-DAG: call i32 @__tgt_target_mapper(i64 {{.+}}, i8* {{.+}}, i32 1, 
i8** [[BPGEP:%[0-9]+]], i8** [[PGEP:%[0-9]+]], {{.+}}[[SIZES]]{{.+}}, 
{{.+}}[[TYPES]]{{.+}}, i8** null)
+  // CK3-DAG: [[BPGEP]] = getelementptr inbounds {{.+}}[[BPS:%[^,]+]], i32 0, 
i32 0
+  // CK3-DAG: [[PGEP]] = getelementptr inbounds {{.+}}[[PS:%[^,]+]], i32 0, 
i32 0
+  // CK3-DAG: [[BP1:%.+]] = getelementptr inbounds {{.+}}[[BPS]], i32 0, i32 0
+  // CK3-DAG: [[P1:%.+]] = getelementptr inbounds {{.+}}[[PS]], i32 0, i32 0
+  // CK3-DAG: [[CBP1:%.+]] = bitcast i8** [[BP1]] to double***
+  // CK3-DAG: [[CP1:%.+]] = bitcast i8** [[P1]] to double***
+  // CK3-DAG: store double** [[PTR:%.+]], double*** [[CBP1]]
+  // CK3-DAG: store double** [[PTR]], double*** [[CP1]]
+
+  // CK3: call void [[KERNEL:@.+]](double** [[PTR]])
+#pragma omp target is_device_ptr(ptr)
+  *ptr = 0;
+}
+#endif
 #endif
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -8497,10 +8497,12 @@
 if (DevPointersMap.count(VD)) {
   CombinedInfo.BasePointers.emplace_back(Arg, VD);
   CombinedInfo.Pointers.push_back(Arg);
-  CombinedInfo.Sizes.push_back(
-  
CGF.Builder.CreateIntCast(CGF.getTypeSize(CGF.getContext().VoidPtrTy),
-CGF.Int64Ty, /*isSigned=*/true));
-  CombinedInfo.Types.push_back(OMP_MAP_LITERAL | OMP_MAP_TARGET_PARAM);
+  CombinedInfo.Sizes.push_back(CGF.Builder.CreateIntCast(
+  CGF.getTypeSize(CGF.getContext().VoidPtrTy), CGF.Int64Ty,
+  /*isSigned=*/true));
+  CombinedInfo.Types.push_back(
+  (Cap->capturesVariable() ? OMP_MAP_TO : 

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

> This is not ideal, since it makes the

calculations char size dependent, and breaks for sizes that
are not a multiple of the char size.

How can we have a non-bitfield member whose size is not a multiple of the size 
of a char?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85191

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

[PATCH] D84412: [clang][Driver] Don't hardcode --as-needed/--no-as-needed on Illumos

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

#clang  does not have many people. You 
might need to add people explicitly..




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:633
 
+static const char *getAsNeededOption(const ToolChain , bool ignore) {
+  // FIXME: Need to handle GNU ld on Solaris.

`ignore` seems strange. How about `start`?



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:634
+static const char *getAsNeededOption(const ToolChain , bool ignore) {
+  // FIXME: Need to handle GNU ld on Solaris.
+  if (TC.getTriple().isOSSolaris())

Can you improve the comment to mention that this is to work around illumnos ld?



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:636
+  if (TC.getTriple().isOSSolaris())
+return ignore ? "-zignore" : "-zrecord";
+  else

I'll assume that `-zignore` has semantics similar to --as-needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84412

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

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1841
   auto setDeclInfo = [&](bool IsIncompleteArrayType) {
-TypeInfo TI = Context.getTypeInfo(D->getType());
-FieldAlign = Context.toCharUnitsFromBits(TI.Align);
+auto TI = Context.getTypeInfoInChars(D->getType());
+FieldAlign = TI.second;

Xiangling_L wrote:
> In most cases, `getTypeInfoInChars` invokes `getTypeInfo` underneath. So to 
> make people be careful about this, I would suggest to leave a comment 
> explaining/claiming we have to call `getTypeInfoInChars` here. And also maybe 
> adding a testcase to guard the scenario you were talking about would be 
> helpful to prevent someone to use `getTypeInfo` here in the future.
I can do that.

I honestly don't think it would be a bad idea to add an assertion to 
toCharUnitsFromBits that checks for non-bytesize-multiple amounts. I wonder how 
much would fail if I did that, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85191

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

[clang] 6d67506 - [clang][BPF] support type exist/size and enum exist/value relocations

[Yonghong Song via cfe-commits]

Author: Yonghong Song
Date: 2020-08-04T08:39:53-07:00
New Revision: 6d6750696400e7ce988d66a1a00e1d0cb32815f8

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

LOG: [clang][BPF] support type exist/size and enum exist/value relocations

This patch added the following additional compile-once
run-everywhere (CO-RE) relocations:
  - existence/size of typedef, struct/union or enum type
  - enum value and enum value existence

These additional relocations will make CO-RE bpf programs more
adaptive for potential kernel internal data structure changes.

For existence/size relocations, the following two code patterns
are supported:
  1. uint32_t __builtin_preserve_type_info(*( *)0, flag);
  2.  var;
 uint32_t __builtin_preserve_field_info(var, flag);
flag = 0 for existence relocation and flag = 1 for size relocation.

For enum value existence and enum value relocations, the following code
pattern is supported:
  uint64_t __builtin_preserve_enum_value(*( *),
 flag);
flag = 0 means existence relocation and flag = 1 for enum value.
relocation. In the above  can be an enum type or
a typedef to enum type. The  needs to be an enumerator
value from the same enum type. The return type is uint64_t to
permit potential 64bit enumerator values.

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

Added: 
clang/test/CodeGen/builtins-bpf-preserve-field-info-3.c
clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c

Modified: 
clang/include/clang/Basic/BuiltinsBPF.def
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/builtins-bpf.c
llvm/include/llvm/IR/IntrinsicsBPF.td

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsBPF.def 
b/clang/include/clang/Basic/BuiltinsBPF.def
index 237e9dc8784b..04b45a52cbe7 100644
--- a/clang/include/clang/Basic/BuiltinsBPF.def
+++ b/clang/include/clang/Basic/BuiltinsBPF.def
@@ -23,5 +23,11 @@ TARGET_BUILTIN(__builtin_preserve_field_info, "Ui.", "t", "")
 // Get BTF type id.
 TARGET_BUILTIN(__builtin_btf_type_id, "Ui.", "t", "")
 
+// Get type information.
+TARGET_BUILTIN(__builtin_preserve_type_info, "Ui.", "t", "")
+
+// Preserve enum value.
+TARGET_BUILTIN(__builtin_preserve_enum_value, "Li.", "t", "")
+
 #undef BUILTIN
 #undef TARGET_BUILTIN

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 054b81c4a72b..7bcff3eb4d8c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10865,6 +10865,14 @@ def err_preserve_field_info_not_const: Error<
   "__builtin_preserve_field_info argument %0 not a constant">;
 def err_btf_type_id_not_const: Error<
   "__builtin_btf_type_id argument %0 not a constant">;
+def err_preserve_type_info_invalid : Error<
+  "__builtin_preserve_type_info argument %0 invalid">;
+def err_preserve_type_info_not_const: Error<
+  "__builtin_preserve_type_info argument %0 not a constant">;
+def err_preserve_enum_value_invalid : Error<
+  "__builtin_preserve_enum_value argument %0 invalid">;
+def err_preserve_enum_value_not_const: Error<
+  "__builtin_preserve_enum_value argument %0 not a constant">;
 
 def err_bit_cast_non_trivially_copyable : Error<
   "__builtin_bit_cast %select{source|destination}0 type must be trivially 
copyable">;

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 2ef164b8b65a..18911184aa41 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -10921,9 +10921,16 @@ Value 
*CodeGenFunction::EmitAArch64BuiltinExpr(unsigned BuiltinID,
 Value *CodeGenFunction::EmitBPFBuiltinExpr(unsigned BuiltinID,
const CallExpr *E) {
   assert((BuiltinID == BPF::BI__builtin_preserve_field_info ||
-  BuiltinID == BPF::BI__builtin_btf_type_id) &&
+  BuiltinID == BPF::BI__builtin_btf_type_id ||
+  BuiltinID == BPF::BI__builtin_preserve_type_info ||
+  BuiltinID == BPF::BI__builtin_preserve_enum_value) &&
  "unexpected BPF builtin");
 
+  // A sequence number, injected into IR builtin functions, to
+  // prevent CSE given the only 
diff erence of the funciton
+  // may just be the debuginfo metadata.
+  static uint32_t BuiltinSeqNum;
+
   switch (BuiltinID) {
   default:
 llvm_unreachable("Unexpected BPF builtin");
@@ -11016,6 +11023,63 @@ Value *CodeGenFunction::EmitBPFBuiltinExpr(unsigned 
BuiltinID,
 Fn->setMetadata(LLVMContext::MD_preserve_access_index, DbgInfo);
 return Fn;
   }
+  case BPF::BI__builtin_preserve_type_info: {
+if (!getDebugInfo()) {
+  CGM.Error(E->getExprLoc(), "using 

[PATCH] D83242: [clang][BPF] support type exist/size and enum exist/value relocations

[Yonghong Song via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6d6750696400: [clang][BPF] support type exist/size and enum 
exist/value relocations (authored by yonghong-song).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83242

Files:
  clang/include/clang/Basic/BuiltinsBPF.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-bpf-preserve-field-info-3.c
  clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c
  clang/test/Sema/builtins-bpf.c
  llvm/include/llvm/IR/IntrinsicsBPF.td

Index: llvm/include/llvm/IR/IntrinsicsBPF.td
===
--- llvm/include/llvm/IR/IntrinsicsBPF.td
+++ llvm/include/llvm/IR/IntrinsicsBPF.td
@@ -26,4 +26,10 @@
   def int_bpf_btf_type_id : GCCBuiltin<"__builtin_bpf_btf_type_id">,
   Intrinsic<[llvm_i32_ty], [llvm_any_ty, llvm_any_ty, llvm_i64_ty],
   [IntrNoMem]>;
+  def int_bpf_preserve_type_info : GCCBuiltin<"__builtin_bpf_preserve_type_info">,
+  Intrinsic<[llvm_i32_ty], [llvm_i32_ty, llvm_i64_ty],
+  [IntrNoMem]>;
+  def int_bpf_preserve_enum_value : GCCBuiltin<"__builtin_bpf_preserve_enum_value">,
+  Intrinsic<[llvm_i64_ty], [llvm_i32_ty, llvm_ptr_ty, llvm_i64_ty],
+  [IntrNoMem]>;
 }
Index: clang/test/Sema/builtins-bpf.c
===
--- clang/test/Sema/builtins-bpf.c
+++ clang/test/Sema/builtins-bpf.c
@@ -1,7 +1,28 @@
 // RUN: %clang_cc1 -x c -triple bpf-pc-linux-gnu -dwarf-version=4 -fsyntax-only -verify %s
 
-struct s { int a; int b[4]; int c:1; };
-union u { int a; int b[4]; int c:1; };
+struct s {
+  int a;
+  int b[4];
+  int c:1;
+};
+union u {
+  int a;
+  int b[4];
+  int c:1;
+};
+typedef struct {
+  int a;
+  int b;
+} __t;
+typedef int (*__f)(void);
+enum AA {
+  VAL1 = 10,
+  VAL2 = 0x8000UL,
+};
+typedef enum {
+  VAL10 = 10,
+  VAL11 = 11,
+} __BB;
 
 unsigned invalid1(const int *arg) {
   return __builtin_preserve_field_info(arg, 1); // expected-error {{__builtin_preserve_field_info argument 1 not a field access}}
@@ -46,3 +67,38 @@
 unsigned invalid11(struct s *arg, int info_kind) {
   return __builtin_preserve_field_info(arg->a, info_kind); // expected-error {{__builtin_preserve_field_info argument 2 not a constant}}
 }
+
+unsigned valid12() {
+  const struct s t;
+  return __builtin_preserve_type_info(t, 0) +
+ __builtin_preserve_type_info(*(struct s *)0, 1);
+}
+
+unsigned valid13() {
+  __t t;
+  return __builtin_preserve_type_info(t, 1) +
+ __builtin_preserve_type_info(*(__t *)0, 0);
+}
+
+unsigned valid14() {
+  enum AA t;
+  return __builtin_preserve_type_info(t, 0) +
+ __builtin_preserve_type_info(*(enum AA *)0, 1);
+}
+
+unsigned valid15() {
+  return __builtin_preserve_enum_value(*(enum AA *)VAL1, 1) +
+ __builtin_preserve_enum_value(*(enum AA *)VAL2, 1);
+}
+
+unsigned invalid16() {
+  return __builtin_preserve_enum_value(*(enum AA *)0, 1); // expected-error {{__builtin_preserve_enum_value argument 1 invalid}}
+}
+
+unsigned invalid17() {
+  return __builtin_preserve_enum_value(*(enum AA *)VAL10, 1); // expected-error {{__builtin_preserve_enum_value argument 1 invalid}}
+}
+
+unsigned invalid18(struct s *arg) {
+  return __builtin_preserve_type_info(arg->a + 2, 0); // expected-error {{__builtin_preserve_type_info argument 1 invalid}}
+}
Index: clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c
===
--- /dev/null
+++ clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c
@@ -0,0 +1,32 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+#define _(x, y) (__builtin_preserve_enum_value((x), (y)))
+
+enum AA {
+  VAL1 = 2,
+  VAL2 = 0x8000UL,
+};
+typedef enum { VAL10 = -2, VAL11 = 0x8000, }  __BB;
+
+unsigned unit1() {
+  return _(*(enum AA *)VAL1, 0) + _(*(__BB *)VAL10, 1);
+}
+
+unsigned unit2() {
+  return _(*(enum AA *)VAL2, 0) + _(*(__BB *)VAL11, 1);
+}
+
+// CHECK: @0 = private unnamed_addr constant [7 x i8] c"VAL1:2\00", align 1
+// CHECK: @1 = private unnamed_addr constant [9 x i8] c"VAL10:-2\00", align 1
+// CHECK: @2 = private unnamed_addr constant [17 x i8] c"VAL2:-2147483648\00", align 1
+// CHECK: @3 = private unnamed_addr constant [17 x i8] c"VAL11:4294934528\00", align 1
+
+// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 0, i8* getelementptr inbounds ([7 x i8], [7 x i8]* @0, i32 0, i32 0), i64 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[ENUM_AA:[0-9]+]]
+// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 1, i8* getelementptr inbounds ([9 x i8], [9 x i8]* @1, i32 0, i32 0), i64 1), !dbg 

[clang] 5e0a9dc - Separate code-block tag with a newline to fix code snippet html output

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-08-04T16:36:00+01:00
New Revision: 5e0a9dc0ad7704b7c49995101629010f5ff98cd2

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

LOG: Separate code-block tag with a newline to fix code snippet html output

Added: 


Modified: 
clang/include/clang/Basic/AttrDocs.td

Removed: 




diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index b9fcf9af323b..76a075a97ee1 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -1084,6 +1084,7 @@ not made control-dependent on any additional values, 
e.g., unrolling a loop
 executed by all work items.
 
 Sample usage:
+
 .. code-block:: c
 
   void convfunc(void) __attribute__((convergent));



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

[PATCH] D84844: [OpenMP] Ensure testing for versions 4.5 and default - Part 1

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam added a comment.

Next two (independent) parts are: D85150  and 
D85214 . More to come.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84844

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

[PATCH] D82575: [OpenMP] OpenMP Clang tests without 50 version string after upgrading to 5.0

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam abandoned this revision.
saiislam added a comment.

Abandoning in favor of D85214 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82575

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

[PATCH] D85214: [OpenMP] Ensure testing for versions 4.5 and default - Part 3

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam added a comment.

This patch updates 101 out 320 test files. Remaining will be posted as a 
separate patch to ease review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85214

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

[PATCH] D85213: Use --dsym-dir when compiling with a compiler that supports it

[Daniel Sanders via Phabricator via cfe-commits]

dsanders created this revision.
dsanders added reviewers: JDevlieghere, beanz, bogner.
Herald added subscribers: cfe-commits, dang, mgorny.
Herald added projects: clang, LLVM.
dsanders requested review of this revision.

As part of this, a couple tweaks to --dsym-dir have been made. It now starts
with a double dash since CMake is unable to detect it's availability otherwise.
clang's that lack it ended up passing the check anyway because of the -d group
accepting anything. Also added the --dsym-dir= variant as the behaviour of
Joined surprised me by including the = in the value otherwise.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85213

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/darwin-dsymutil.c
  llvm/cmake/modules/AddLLVM.cmake


Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1981,21 +1981,29 @@
 set_property(TARGET ${name} APPEND_STRING PROPERTY
   LINK_FLAGS " ${empty_src}")
 if(LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR)
-  # clang can currently only emit dSYM's to the same directory as the 
output
-  # binary. Workaround this by moving it after the build.
-  set(output_name "$.${file_ext}")
-  set(output_path 
"${LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR}/${output_name}")
-  add_custom_command(TARGET ${name} POST_BUILD
-COMMAND ${CMAKE_COMMAND} -E make_directory 
"${LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR}"
-# Remove any old versions if present
-COMMAND ${CMAKE_COMMAND} -E rm "-rf" "${output_path}"
-# Move the dSYM clang emitted next to the output binary where we want 
it
-# to be.
-COMMAND ${CMAKE_COMMAND}
--DFROM="$.${file_ext}"
--DTO="${output_path}"
--P ${LLVM_CMAKE_PATH}/MoveIfExists.cmake
-  )
+  check_cxx_compiler_flag("--dsym-dir=foo" HAS_DSYM_DIR)
+  if (HAS_DSYM_DIR)
+set_property(TARGET ${name} APPEND_STRING PROPERTY
+ LINK_FLAGS " 
--dsym-dir=${LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR}")
+add_custom_command(TARGET ${name} PRE_BUILD
+  COMMAND ${CMAKE_COMMAND} -E make_directory 
"${LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR}")
+  else()
+# clang can currently only emit dSYM's to the same directory as the 
output
+# binary. Workaround this by moving it after the build.
+set(output_name "$.${file_ext}")
+set(output_path 
"${LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR}/${output_name}")
+add_custom_command(TARGET ${name} POST_BUILD
+  COMMAND ${CMAKE_COMMAND} -E make_directory 
"${LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR}"
+  # Remove any old versions if present
+  COMMAND ${CMAKE_COMMAND} -E rm "-rf" "${output_path}"
+  # Move the dSYM clang emitted next to the output binary where we 
want it
+  # to be.
+  COMMAND ${CMAKE_COMMAND}
+  -DFROM="$.${file_ext}"
+  -DTO="${output_path}"
+  -P ${LLVM_CMAKE_PATH}/MoveIfExists.cmake
+)
+  endif()
 endif()
 add_custom_command(TARGET ${name} POST_BUILD
   COMMAND ${strip_command}
Index: clang/test/Driver/darwin-dsymutil.c
===
--- clang/test/Driver/darwin-dsymutil.c
+++ clang/test/Driver/darwin-dsymutil.c
@@ -35,7 +35,7 @@
 // RUN:   -check-prefix=CHECK-OUTPUT-NAME < %t %s
 //
 // RUN: %clang -target x86_64-apple-darwin10 -ccc-print-bindings \
-// RUN:   -o bar/foo -dsym-dir external %s -g 2> %t
+// RUN:   -o bar/foo --dsym-dir external %s -g 2> %t
 // RUN: FileCheck -Doutfile=bar/foo -Ddsymfile=external/foo.dSYM \
 // RUN:   -check-prefix=CHECK-OUTPUT-NAME < %t %s
 //
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -677,9 +677,10 @@
   HelpText<"Filename to write DOT-formatted header dependencies to">;
 def module_dependency_dir : Separate<["-"], "module-dependency-dir">,
   Flags<[CC1Option]>, HelpText<"Directory to dump module dependencies to">;
-def dsym_dir : JoinedOrSeparate<["-"], "dsym-dir">,
+def dsym_dir : Separate<["--"], "dsym-dir">,
   Flags<[DriverOption, RenderAsInput]>,
   HelpText<"Directory to output dSYM's (if any) to">, MetaVarName<"">;
+def dsym_dir_EQ : Joined<["--"], "dsym-dir=">, Alias;
 def dumpmachine : Flag<["-"], "dumpmachine">;
 def dumpspecs : Flag<["-"], "dumpspecs">, Flags<[Unsupported]>;
 def dumpversion : Flag<["-"], "dumpversion">;


Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1981,21 +1981,29 @@
 set_property(TARGET ${name} 

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

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

Szelethus accepted this revision.
Szelethus added a reviewer: gamesh411.
Szelethus added a comment.

LGTM on my end, but please wait for approval from either @gamesh411 or @NoQ.

In D77150#2191049 , 
@baloghadamsoftware wrote:

> However, I think we should create a category "advanced" for options which 
> affect the internal modeling. Our users are programmers and the advanced 
> among them should see this option (and others similar to this one).

I think that is a discussion for another time, and definitely on cfe-dev.


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

https://reviews.llvm.org/D77150

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

[PATCH] D75181: [AArch64] Handle BTI/PAC in case of generated functions.

[Daniel Kiss via Phabricator via cfe-commits]

danielkiss added a comment.

Would it be better to add a new value to `"sign-return-address"` as `"none"`? I 
don't see any other alternative option, I'm open to any other idea.


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

https://reviews.llvm.org/D75181

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

[clang] 0a8ac91 - Permit nowthrow and nonnull with multiversioning.

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2020-08-04T07:40:27-07:00
New Revision: 0a8ac91a084504929b1ef4ec1fee693455bd796d

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

LOG: Permit nowthrow and nonnull with multiversioning.

Some shipped versions of stdlib.h use nonnull and nothrow with function
multiversioning.  Support these, as they are generally harmless.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDecl.cpp
clang/test/Sema/attr-cpuspecific.c
clang/test/Sema/attr-target-mv.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 288e8232ca74..054b81c4a72b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10754,6 +10754,9 @@ def err_multiversion_noproto : Error<
 def err_multiversion_disallowed_other_attr : Error<
   "attribute '%select{target|cpu_specific|cpu_dispatch}0' multiversioning 
cannot be combined"
   " with attribute %1">;
+def err_multiversion_mismatched_attrs
+: Error<"attributes on multiversioned functions must all match, attribute "
+"%0 %select{is missing|has 
diff erent arguments}1">;
 def err_multiversion_
diff  : Error<
   "multiversioned function declaration has a 
diff erent %select{calling convention"
   "|return type|constexpr specification|inline specification|storage class|"

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index ba05b0d32cf4..77e15f187e53 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -10029,11 +10029,16 @@ static bool CheckMultiVersionValue(Sema , const 
FunctionDecl *FD) {
 // multiversion functions.
 static bool AttrCompatibleWithMultiVersion(attr::Kind Kind,
MultiVersionKind MVType) {
+  // Note: this list/diagnosis must match the list in
+  // checkMultiversionAttributesAllSame.
   switch (Kind) {
   default:
 return false;
   case attr::Used:
 return MVType == MultiVersionKind::Target;
+  case attr::NonNull:
+  case attr::NoThrow:
+return true;
   }
 }
 
@@ -10201,8 +10206,6 @@ static bool CheckMultiVersionAdditionalRules(Sema , 
const FunctionDecl *OldFD,
   MVType == MultiVersionKind::CPUDispatch ||
   MVType == MultiVersionKind::CPUSpecific;
 
-  // For now, disallow all other attributes.  These should be opt-in, but
-  // an analysis of all of them is a future FIXME.
   if (CausesMV && OldFD &&
   checkNonMultiVersionCompatAttributes(S, OldFD, NewFD, MVType))
 return true;

diff  --git a/clang/test/Sema/attr-cpuspecific.c 
b/clang/test/Sema/attr-cpuspecific.c
index e32c7a22894d..9cfeef8a2356 100644
--- a/clang/test/Sema/attr-cpuspecific.c
+++ b/clang/test/Sema/attr-cpuspecific.c
@@ -115,3 +115,5 @@ int use3(void) {
 
 // expected-warning@+1 {{CPU list contains duplicate entries; attribute 
ignored}}
 int __attribute__((cpu_dispatch(pentium_iii, pentium_iii_no_xmm_regs))) 
dupe_p3(void);
+
+void __attribute__((cpu_specific(atom), nothrow, nonnull(1))) 
addtl_attrs(int*);

diff  --git a/clang/test/Sema/attr-target-mv.c 
b/clang/test/Sema/attr-target-mv.c
index 33a2c4fa54eb..3f072b19083f 100644
--- a/clang/test/Sema/attr-target-mv.c
+++ b/clang/test/Sema/attr-target-mv.c
@@ -101,3 +101,10 @@ __vectorcall int 
__attribute__((target("arch=sandybridge")))  
diff _cc(void);
 int __attribute__((target("sse4.2"))) 
diff _ret(void);
 // expected-error@+1 {{multiversioned function declaration has a 
diff erent return type}}
 short __attribute__((target("arch=sandybridge")))  
diff _ret(void);
+
+void __attribute__((target("sse4.2"), nothrow, used, nonnull(1))) 
addtl_attrs5(int*);
+void __attribute__((target("arch=sandybridge"))) addtl_attrs5(int*);
+
+void __attribute__((target("sse4.2"))) addtl_attrs6(int*);
+void __attribute__((target("arch=sandybridge"), nothrow, used, nonnull)) 
addtl_attrs6(int*);
+



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

[clang] 961da69 - Improve diagnostics for disallowed attributes used with multiversioning

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2020-08-04T07:40:27-07:00
New Revision: 961da69d7eafe44411d5ac9719209653d196f9e2

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

LOG: Improve diagnostics for disallowed attributes used with multiversioning

Since we permit using SOME attributes (at the moment, just 1) with
multiversioning, we should improve the message as it still implies that
no attributes should be combined with multiversioning.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDecl.cpp
clang/test/Sema/attr-cpuspecific.c
clang/test/Sema/attr-target-mv.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 91112860a2d0..288e8232ca74 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10751,9 +10751,9 @@ def err_multiversion_duplicate : Error<
   "multiversioned function redeclarations require identical target 
attributes">;
 def err_multiversion_noproto : Error<
   "multiversioned function must have a prototype">;
-def err_multiversion_no_other_attrs : Error<
+def err_multiversion_disallowed_other_attr : Error<
   "attribute '%select{target|cpu_specific|cpu_dispatch}0' multiversioning 
cannot be combined"
-  " with other attributes">;
+  " with attribute %1">;
 def err_multiversion_
diff  : Error<
   "multiversioned function declaration has a 
diff erent %select{calling convention"
   "|return type|constexpr specification|inline specification|storage class|"

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 531c2801bf92..ba05b0d32cf4 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -10037,23 +10037,37 @@ static bool AttrCompatibleWithMultiVersion(attr::Kind 
Kind,
   }
 }
 
-static bool HasNonMultiVersionAttributes(const FunctionDecl *FD,
- MultiVersionKind MVType) {
+static bool checkNonMultiVersionCompatAttributes(Sema ,
+ const FunctionDecl *FD,
+ const FunctionDecl *CausedFD,
+ MultiVersionKind MVType) {
+  bool IsCPUSpecificCPUDispatchMVType =
+  MVType == MultiVersionKind::CPUDispatch ||
+  MVType == MultiVersionKind::CPUSpecific;
+  const auto Diagnose = [FD, CausedFD, IsCPUSpecificCPUDispatchMVType](
+Sema , const Attr *A) {
+S.Diag(FD->getLocation(), diag::err_multiversion_disallowed_other_attr)
+<< IsCPUSpecificCPUDispatchMVType << A;
+if (CausedFD)
+  S.Diag(CausedFD->getLocation(), diag::note_multiversioning_caused_here);
+return true;
+  };
+
   for (const Attr *A : FD->attrs()) {
 switch (A->getKind()) {
 case attr::CPUDispatch:
 case attr::CPUSpecific:
   if (MVType != MultiVersionKind::CPUDispatch &&
   MVType != MultiVersionKind::CPUSpecific)
-return true;
+return Diagnose(S, A);
   break;
 case attr::Target:
   if (MVType != MultiVersionKind::Target)
-return true;
+return Diagnose(S, A);
   break;
 default:
   if (!AttrCompatibleWithMultiVersion(A->getKind(), MVType))
-return true;
+return Diagnose(S, A);
   break;
 }
   }
@@ -10189,16 +10203,12 @@ static bool CheckMultiVersionAdditionalRules(Sema , 
const FunctionDecl *OldFD,
 
   // For now, disallow all other attributes.  These should be opt-in, but
   // an analysis of all of them is a future FIXME.
-  if (CausesMV && OldFD && HasNonMultiVersionAttributes(OldFD, MVType)) {
-S.Diag(OldFD->getLocation(), diag::err_multiversion_no_other_attrs)
-<< IsCPUSpecificCPUDispatchMVType;
-S.Diag(NewFD->getLocation(), diag::note_multiversioning_caused_here);
+  if (CausesMV && OldFD &&
+  checkNonMultiVersionCompatAttributes(S, OldFD, NewFD, MVType))
 return true;
-  }
 
-  if (HasNonMultiVersionAttributes(NewFD, MVType))
-return S.Diag(NewFD->getLocation(), diag::err_multiversion_no_other_attrs)
-   << IsCPUSpecificCPUDispatchMVType;
+  if (checkNonMultiVersionCompatAttributes(S, NewFD, nullptr, MVType))
+return true;
 
   // Only allow transition to MultiVersion if it hasn't been used.
   if (OldFD && CausesMV && OldFD->isUsed(false))

diff  --git a/clang/test/Sema/attr-cpuspecific.c 
b/clang/test/Sema/attr-cpuspecific.c
index ae86742ca081..e32c7a22894d 100644
--- a/clang/test/Sema/attr-cpuspecific.c
+++ b/clang/test/Sema/attr-cpuspecific.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu  -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu  

[PATCH] D84637: [AST] Enhance the const expression evaluator to support error-dependent exprs.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ExprConstant.cpp:4961
 }
+if (IS->getCond()->isValueDependent())
+  return EvaluateDependentExpr(IS->getCond(), Info);

The `if` stmt (the same to `while`, `for`) is tricky -- if the condition is 
value-dependent, then we don't know which branch we should run into.

- returning a ESR_Succeeded may lead to a bogus diagnostic ("reach the end of 
the function");
- returning a ESR_Failed may lead to a bogus diagnostic ("never produce a 
constexpr"); 

I guess what we want is to stop the evaluation, and indicate that we hit a 
value-dependent expression and we don't know how to evaluate it, but still 
treat the constexpr function as potential constexpr (but no extra diagnostics 
being emitted), but the current `EvalStmtResult` is not sufficient, maybe we 
need a new enum.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84637

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

[PATCH] D83338: [PowerPC][Power10] Implemented Vector Shift Builtins

[Amy Kwan via Phabricator via cfe-commits]

amyk added inline comments.



Comment at: clang/lib/Headers/altivec.h:17217
+
+/* vs[l | r | ra] */
+static __inline__ vector unsigned __int128 __ATTRS_o_ai

Add a space after this comment.



Comment at: clang/lib/Headers/altivec.h:17227
+vec_sl(vector signed __int128 __a, vector unsigned __int128 __b) {
+  // return (vector signed __int128)vec_sl((vector unsigned __int128)__a, __b);
+  return __a << (__b %

Please remove any commented code.



Comment at: clang/lib/Headers/altivec.h:17243
+  // return (vector signed __int128)vec_sr((vector unsigned __int128)__a, __b);
+  return (vector signed __int128)(
+((vector unsigned __int128) __a) >>

Could you please fix the indentation of the returns to make them all consistent?



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:1100
+
+if (Subtarget.isISA3_1()) {
+  setOperationAction(ISD::SRA, MVT::v1i128, Legal);

No brackets are needed here.

Also, I think it might make sense to move this block into the previous 
`hasP9Altivec` condition since in there it has:
```
  setOperationAction(ISD::SHL, MVT::v1i128, Legal);
  setOperationAction(ISD::SRL, MVT::v1i128, Legal);
```



Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:1141
+/* Vector shifts for ISA3_1 */
+let Predicates = [IsISA3_1] in {
+  def : Pat<(v1i128 (shl v1i128:$VRA, v1i128:$VRB)),

There is no need to make a new predicate block, you can put these anonymous 
patterns in the block above.



Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:1144
+(v1i128 (VSLQ v1i128:$VRA, v1i128:$VRB))>;
+  def : Pat<(v1i128 (PPCshl v1i128:$VRA, v1i128:$VRB)),
+(v1i128 (VSLQ v1i128:$VRA, v1i128:$VRB))>;

I noticed that we have patterns for the PPCISD nodes, but I think no tests to 
show these patterns?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83338

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

[PATCH] D85146: [SyntaxTree] Fix crash on pointer to member function

[Eduardo Caldas via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8ce15f7eeb12: [SyntaxTree] Fix crash on pointer to member 
function (authored by eduucaldas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85146

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -4074,6 +4074,99 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, MemberFunctionPointer) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+struct X {
+  struct Y {};
+};
+void (X::*xp)();
+void (X::**xpp)(const int*);
+// FIXME: Generate the right syntax tree for this type,
+// i.e. create a syntax node for the outer member pointer
+void (X::Y::*xyp)(const int*, char);
+)cpp",
+  R"txt(
+*: TranslationUnit
+|-SimpleDeclaration
+| |-struct
+| |-X
+| |-{
+| |-SimpleDeclaration
+| | |-struct
+| | |-Y
+| | |-{
+| | |-}
+| | `-;
+| |-}
+| `-;
+|-SimpleDeclaration
+| |-void
+| |-SimpleDeclarator
+| | |-ParenDeclarator
+| | | |-(
+| | | |-MemberPointer
+| | | | |-X
+| | | | |-::
+| | | | `-*
+| | | |-xp
+| | | `-)
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   `-)
+| `-;
+|-SimpleDeclaration
+| |-void
+| |-SimpleDeclarator
+| | |-ParenDeclarator
+| | | |-(
+| | | |-MemberPointer
+| | | | |-X
+| | | | |-::
+| | | | `-*
+| | | |-*
+| | | |-xpp
+| | | `-)
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   |-SimpleDeclaration
+| |   | |-const
+| |   | |-int
+| |   | `-SimpleDeclarator
+| |   |   `-*
+| |   `-)
+| `-;
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-ParenDeclarator
+  | | |-(
+  | | |-X
+  | | |-::
+  | | |-MemberPointer
+  | | | |-Y
+  | | | |-::
+  | | | `-*
+  | | |-xyp
+  | | `-)
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-const
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   `-*
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | `-char
+  |   `-)
+  `-;
+)txt"));
+}
+
 TEST_P(SyntaxTreeTest, ComplexDeclarator) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -939,6 +939,8 @@
 return true;
   }
 
+  // FIXME: Deleting the `TraverseParenTypeLoc` override doesn't change test
+  // results. Find test coverage or remove it.
   bool TraverseParenTypeLoc(ParenTypeLoc L) {
 // We reverse order of traversal to get the proper syntax structure.
 if (!WalkUpFromParenTypeLoc(L))
@@ -987,6 +989,16 @@
 return WalkUpFromFunctionTypeLoc(L);
   }
 
+  bool TraverseMemberPointerTypeLoc(MemberPointerTypeLoc L) {
+// In the source code "void (Y::*mp)()" `MemberPointerTypeLoc` corresponds
+// to "Y::*" but it points to a `ParenTypeLoc` that corresponds to
+// "(Y::*mp)" We thus reverse the order of traversal to get the proper
+// syntax structure.
+if (!WalkUpFromMemberPointerTypeLoc(L))
+  return false;
+return TraverseTypeLoc(L.getPointeeLoc());
+  }
+
   bool WalkUpFromMemberPointerTypeLoc(MemberPointerTypeLoc L) {
 auto SR = L.getLocalSourceRange();
 Builder.foldNode(Builder.getRange(SR),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits