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

2021-08-04 Thread Steven Wan via Phabricator via cfe-commits
stevewan updated this revision to Diff 364359.
stevewan added a comment.

Add test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/Layout/aix-alignof-align-and-pack-attr.cpp

Index: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+namespace test1 {
+struct __attribute__((__aligned__(2))) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test1{{.*}}c{{.*}} = global %"struct.test1::C" zeroinitializer, align 8
+} // namespace test1
+
+namespace test2 {
+struct __attribute__((__aligned__(2), packed)) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test2{{.*}}c{{.*}} = global %"struct.test2::C" zeroinitializer, align 2
+} // namespace test2
+
+namespace test3 {
+struct C {
+  struct CC {
+long double ld;
+  } __attribute__((aligned(2))) cc;
+} c;
+
+// CHECK: @{{.*}}test3{{.*}}c{{.*}} = global %"struct.test3::C" zeroinitializer, align 8
+} // namespace test3
+
+namespace test4 {
+struct C {
+  struct CC {
+long double ld;
+  } __attribute__((aligned(2), packed)) cc;
+} c;
+
+// CHECK: @{{.*}}test4{{.*}}c{{.*}} = global %"struct.test4::C" zeroinitializer, align 2
+} // namespace test4
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1961,6 +1961,12 @@
 }
   }
 
+  const Type *Ty = D->getType()->getBaseElementTypeUnsafe();
+  // When used as part of a typedef, the 'aligned' attribute can be used to
+  // decrease alignment.
+  bool AlignAttrCanDecreaseAlignment =
+  AlignIsRequired && Ty->getAs() != nullptr;
+
   // The AIX `power` alignment rules apply the natural alignment of the
   // "first member" if it is of a floating-point data type (or is an aggregate
   // whose recursively "first" member or element is such a type). The alignment
@@ -1971,7 +1977,7 @@
   // and zero-width bit-fields count as prior members; members of empty class
   // types marked `no_unique_address` are not considered to be prior members.
   CharUnits PreferredAlign = FieldAlign;
-  if (DefaultsToAIXPowerAlignment && !AlignIsRequired &&
+  if (DefaultsToAIXPowerAlignment && !AlignAttrCanDecreaseAlignment &&
   (FoundFirstNonOverlappingEmptyFieldForAIX || IsNaturalAlign)) {
 auto performBuiltinTypeAlignmentUpgrade = [&](const BuiltinType *BTy) {
   if (BTy->getKind() == BuiltinType::Double ||
@@ -1982,7 +1988,6 @@
   }
 };
 
-const Type *Ty = D->getType()->getBaseElementTypeUnsafe();
 if (const ComplexType *CTy = Ty->getAs()) {
   performBuiltinTypeAlignmentUpgrade(CTy->getElementType()->castAs());
 } else if (const BuiltinType *BTy = Ty->getAs()) {
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2478,11 +2478,16 @@
 return ABIAlign;
 
   if (const auto *RT = T->getAs()) {
-if (TI.AlignIsRequired || RT->getDecl()->isInvalidDecl())
+const RecordDecl *RD = RT->getDecl();
+
+// When used as part of a typedef, or together with a 'packed' attribute,
+// the 'aligned' attribute can be used to decrease alignment.
+if ((TI.AlignIsRequired && T->getAs() != nullptr) ||
+RD->isInvalidDecl())
   return ABIAlign;
 
 unsigned PreferredAlign = static_cast(
-toBits(getASTRecordLayout(RT->getDecl()).PreferredAlignment));
+toBits(getASTRecordLayout(RD).PreferredAlignment));
 assert(PreferredAlign >= ABIAlign &&
"PreferredAlign should be at least as large as ABIAlign.");
 return PreferredAlign;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2021-08-04 Thread Steven Wan via Phabricator via cfe-commits
stevewan updated this revision to Diff 364350.
stevewan marked an inline comment as done.
stevewan added a comment.

Fix the same problem in field layout.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/Layout/aix-alignof-align-and-pack-attr.cpp


Index: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | 
\
+// RUN:   FileCheck %s
+
+namespace test1 {
+struct __attribute__((__aligned__(2))) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test1{{.*}}c{{.*}} = global %"struct.test1::C" 
zeroinitializer, align 8
+} // namespace test1
+
+namespace test2 {
+struct __attribute__((__aligned__(2), packed)) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test2{{.*}}c{{.*}} = global %"struct.test2::C" 
zeroinitializer, align 2
+} // namespace test2
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1961,6 +1961,12 @@
 }
   }
 
+  const Type *Ty = D->getType()->getBaseElementTypeUnsafe();
+  // When used as part of a typedef, the 'aligned' attribute can be used to
+  // decrease alignment.
+  bool AlignAttrCanDecreaseAlignment =
+  AlignIsRequired && Ty->getAs() != nullptr;
+
   // The AIX `power` alignment rules apply the natural alignment of the
   // "first member" if it is of a floating-point data type (or is an aggregate
   // whose recursively "first" member or element is such a type). The alignment
@@ -1971,7 +1977,7 @@
   // and zero-width bit-fields count as prior members; members of empty class
   // types marked `no_unique_address` are not considered to be prior members.
   CharUnits PreferredAlign = FieldAlign;
-  if (DefaultsToAIXPowerAlignment && !AlignIsRequired &&
+  if (DefaultsToAIXPowerAlignment && !AlignAttrCanDecreaseAlignment &&
   (FoundFirstNonOverlappingEmptyFieldForAIX || IsNaturalAlign)) {
 auto performBuiltinTypeAlignmentUpgrade = [&](const BuiltinType *BTy) {
   if (BTy->getKind() == BuiltinType::Double ||
@@ -1982,7 +1988,6 @@
   }
 };
 
-const Type *Ty = D->getType()->getBaseElementTypeUnsafe();
 if (const ComplexType *CTy = Ty->getAs()) {
   
performBuiltinTypeAlignmentUpgrade(CTy->getElementType()->castAs());
 } else if (const BuiltinType *BTy = Ty->getAs()) {
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2478,11 +2478,16 @@
 return ABIAlign;
 
   if (const auto *RT = T->getAs()) {
-if (TI.AlignIsRequired || RT->getDecl()->isInvalidDecl())
+const RecordDecl *RD = RT->getDecl();
+
+// When used as part of a typedef, or together with a 'packed' attribute,
+// the 'aligned' attribute can be used to decrease alignment.
+if ((TI.AlignIsRequired && T->getAs() != nullptr) ||
+RD->isInvalidDecl())
   return ABIAlign;
 
 unsigned PreferredAlign = static_cast(
-toBits(getASTRecordLayout(RT->getDecl()).PreferredAlignment));
+toBits(getASTRecordLayout(RD).PreferredAlignment));
 assert(PreferredAlign >= ABIAlign &&
"PreferredAlign should be at least as large as ABIAlign.");
 return PreferredAlign;


Index: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+namespace test1 {
+struct __attribute__((__aligned__(2))) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test1{{.*}}c{{.*}} = global %"struct.test1::C" zeroinitializer, align 8
+} // namespace test1
+
+namespace test2 {
+struct __attribute__((__aligned__(2), packed)) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test2{{.*}}c{{.*}} = global %"struct.test2::C" zeroinitializer, align 2
+} // namespace test2
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1961,6 +1961,12 @@
 }
   }
 
+  const Type *Ty = D->getType()->getBaseElementTypeUnsafe();
+  // When used as part of a typedef, the 'aligned' attribute can be used to
+  // 

[PATCH] D107420: [sema] Disallow __builtin_mul_overflow under special condition.

2021-08-04 Thread Freddy, Ye via Phabricator via cfe-commits
FreddyYe updated this revision to Diff 364349.
FreddyYe added a comment.

update lit test, clang-format, if condition, diagnostic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107420

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/builtins-overflow.c


Index: clang/test/Sema/builtins-overflow.c
===
--- clang/test/Sema/builtins-overflow.c
+++ clang/test/Sema/builtins-overflow.c
@@ -38,4 +38,22 @@
 _ExtInt(129) result;
 _Bool status = __builtin_mul_overflow(x, y, ); // expected-error 
{{__builtin_mul_overflow does not support signed _ExtInt operands of more than 
128 bits}}
   }
+  {
+_ExtInt(128) x = 1;
+_ExtInt(128) y = 1;
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, ); // expected-error 
{{when __builtin_mul_overflow's result argument points to unsigned 128 bit 
integer, pls make sure input operands won't produce positive (2^127)~(2^128-1) 
from two negative integer value. That's not supported yet.}}
+  }
+  {
+_ExtInt(65) x = 1;
+_ExtInt(64) y = 1;
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, ); // expected-error 
{{when __builtin_mul_overflow's result argument points to unsigned 128 bit 
integer, pls make sure input operands won't produce positive (2^127)~(2^128-1) 
from two negative integer value. That's not supported yet.}}
+  }
+  {
+_ExtInt(64) x = 1;
+_ExtInt(64) y = 1;
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, ); // expected ok
+  }
 }
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -328,6 +328,22 @@
   // Disallow signed ExtIntType args larger than 128 bits to mul function until
   // we improve backend support.
   if (BuiltinID == Builtin::BI__builtin_mul_overflow) {
+const auto LeftTy = TheCall->getArg(0)->getType();
+const auto RightTy = TheCall->getArg(1)->getType();
+const auto ResultPointeeTy =
+TheCall->getArg(2)->getType()->getPointeeType();
+// Input combination below will also emit an integer value larger than
+// 128 bits in the backend, disallow same as above.
+if (!ResultPointeeTy->isSignedIntegerType() &&
+S.getASTContext().getIntWidth(ResultPointeeTy) == 128 &&
+((LeftTy->isSignedIntegerType() ? S.getASTContext().getIntWidth(LeftTy)
+: 0) +
+ (RightTy->isSignedIntegerType()
+  ? S.getASTContext().getIntWidth(RightTy)
+  : 0)) > 128) {
+  return S.Diag(TheCall->getArg(0)->getBeginLoc(),
+diag::err_overflow_builtin_mul_special_condition_max_size);
+}
 for (unsigned I = 0; I < 3; ++I) {
   const auto Arg = TheCall->getArg(I);
   // Third argument will be a pointer.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8348,6 +8348,10 @@
 def err_overflow_builtin_ext_int_max_size : Error<
   "__builtin_mul_overflow does not support signed _ExtInt operands of more "
   "than %0 bits">;
+def err_overflow_builtin_mul_special_condition_max_size : Error<
+  "when __builtin_mul_overflow's result argument points to unsigned 128 bit 
integer, pls make sure "
+  "input operands won't produce positive (2^127)~(2^128-1) from two negative 
integer value. That's "
+  "not supported yet.">;
 
 def err_atomic_load_store_uses_lib : Error<
   "atomic %select{load|store}0 requires runtime support that is not "


Index: clang/test/Sema/builtins-overflow.c
===
--- clang/test/Sema/builtins-overflow.c
+++ clang/test/Sema/builtins-overflow.c
@@ -38,4 +38,22 @@
 _ExtInt(129) result;
 _Bool status = __builtin_mul_overflow(x, y, ); // expected-error {{__builtin_mul_overflow does not support signed _ExtInt operands of more than 128 bits}}
   }
+  {
+_ExtInt(128) x = 1;
+_ExtInt(128) y = 1;
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, ); // expected-error {{when __builtin_mul_overflow's result argument points to unsigned 128 bit integer, pls make sure input operands won't produce positive (2^127)~(2^128-1) from two negative integer value. That's not supported yet.}}
+  }
+  {
+_ExtInt(65) x = 1;
+_ExtInt(64) y = 1;
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, ); // expected-error {{when __builtin_mul_overflow's result argument points to unsigned 128 bit integer, pls make sure input operands won't produce positive 

[PATCH] D107244: [AIX] Define _ARCH_PPC64 macro for 32-bit

2021-08-04 Thread Jake Egan via Phabricator via cfe-commits
Jake-Egan updated this revision to Diff 364342.
Jake-Egan added a comment.

Merge else and if.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107244

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/Preprocessor/init-ppc.c


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -391,6 +391,7 @@
 // PPC-AIX-NOT:#define __64BIT__ 1
 // PPC-AIX:#define _AIX 1
 // PPC-AIX:#define _ARCH_PPC 1
+// PPC-AIX:#define _ARCH_PPC64 1
 // PPC-AIX:#define _BIG_ENDIAN 1
 // PPC-AIX:#define _IBMR2 1
 // PPC-AIX:#define _LONG_LONG 1
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -256,6 +256,9 @@
 Builder.defineMacro("__powerpc64__");
 Builder.defineMacro("__ppc64__");
 Builder.defineMacro("__PPC64__");
+  } else if (getTriple().isOSAIX()) {
+// Also define _ARCH_PPC64 for 32-bit on AIX.
+Builder.defineMacro("_ARCH_PPC64");
   }
 
   // Target properties.


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -391,6 +391,7 @@
 // PPC-AIX-NOT:#define __64BIT__ 1
 // PPC-AIX:#define _AIX 1
 // PPC-AIX:#define _ARCH_PPC 1
+// PPC-AIX:#define _ARCH_PPC64 1
 // PPC-AIX:#define _BIG_ENDIAN 1
 // PPC-AIX:#define _IBMR2 1
 // PPC-AIX:#define _LONG_LONG 1
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -256,6 +256,9 @@
 Builder.defineMacro("__powerpc64__");
 Builder.defineMacro("__ppc64__");
 Builder.defineMacro("__PPC64__");
+  } else if (getTriple().isOSAIX()) {
+// Also define _ARCH_PPC64 for 32-bit on AIX.
+Builder.defineMacro("_ARCH_PPC64");
   }
 
   // Target properties.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107528: [llvm][clang][NFC] updates inline licence info

2021-08-04 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision.
cjdb added reviewers: aaron.ballman, chandlerc, lattner, tonic.
Herald added subscribers: dexonsmith, martong, pengfei, arphaman, kbarton, 
hiraditya, nemanjai, qcolombet, MatzeB.
cjdb requested review of this revision.
Herald added projects: clang, LLDB, LLVM, clang-tools-extra.
Herald added subscribers: cfe-commits, llvm-commits, lldb-commits.

Some files still contained the old University of Illinois Open Source
Licence header. This patch replaces that with the Apache 2 with LLVM
Exception licence.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107528

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.h
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
  clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.h
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
  clang/include/clang/AST/ASTConcept.h
  clang/include/clang/AST/ASTImporterSharedState.h
  clang/include/clang/AST/CurrentSourceLocExprScope.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/Sema/SemaConcept.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/AST/ASTConcept.cpp
  clang/lib/Format/MacroExpander.cpp
  clang/lib/Format/Macros.h
  clang/lib/Index/FileIndexRecord.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/StaticAnalyzer/Core/SMTConstraintManager.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/unittests/Format/TestLexer.h
  clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaTemplateParams.cpp
  lldb/docs/use/python.rst
  lldb/unittests/Symbol/TestLineEntry.cpp
  llvm/include/llvm/Analysis/HeatUtils.h
  llvm/include/llvm/CodeGen/MIRFormatter.h
  llvm/include/llvm/CodeGen/RegAllocCommon.h
  llvm/include/llvm/Support/ExtensibleRTTI.h
  llvm/include/llvm/Support/LICENSE.TXT
  llvm/include/llvm/Support/Signposts.h
  llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
  llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h
  llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/include/llvm/Transforms/Instrumentation/InstrOrderFile.h
  llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h
  llvm/include/llvm/Transforms/Utils/MemoryOpRemark.h
  llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
  llvm/lib/Analysis/HeatUtils.cpp
  llvm/lib/Analysis/InlineSizeEstimatorAnalysis.cpp
  llvm/lib/Analysis/TFUtils.cpp
  llvm/lib/BinaryFormat/MsgPackDocumentYAML.cpp
  llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
  llvm/lib/DebugInfo/GSYM/FileWriter.cpp
  llvm/lib/DebugInfo/GSYM/Range.cpp
  llvm/lib/ExecutionEngine/JITLink/EHFrameSupport.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF.cpp
  llvm/lib/ExecutionEngine/JITLink/JITLink.cpp
  llvm/lib/ExecutionEngine/JITLink/JITLinkMemoryManager.cpp
  llvm/lib/ExecutionEngine/JITLink/MachO.cpp
  llvm/lib/ExecutionEngine/Orc/TargetProcess/RegisterEHFrames.cpp
  llvm/lib/Support/ExtensibleRTTI.cpp
  llvm/lib/Support/Signposts.cpp
  llvm/lib/Target/AArch64/AArch64StackTagging.cpp
  llvm/lib/Target/AArch64/AArch64StackTaggingPreRA.cpp
  llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp
  llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp
  llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp
  llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.h
  llvm/lib/Target/PowerPC/MCTargetDesc/PPCXCOFFStreamer.cpp
  llvm/lib/Target/PowerPC/MCTargetDesc/PPCXCOFFStreamer.h
  llvm/lib/Target/X86/X86InstrKL.td
  llvm/lib/Transforms/Instrumentation/InstrOrderFile.cpp
  llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp
  llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
  llvm/unittests/BinaryFormat/MsgPackDocumentTest.cpp
  llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
  llvm/unittests/Support/ExtensibleRTTITest.cpp

Index: llvm/unittests/Support/ExtensibleRTTITest.cpp
===
--- llvm/unittests/Support/ExtensibleRTTITest.cpp
+++ llvm/unittests/Support/ExtensibleRTTITest.cpp
@@ -1,9 +1,8 @@
 //===-- unittests/ExtensibleRTTITest.cpp - Extensible RTTI Tests --===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 

[PATCH] D107244: [AIX] Define _ARCH_PPC64 macro for 32-bit

2021-08-04 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/PPC.cpp:259
 Builder.defineMacro("__PPC64__");
+  // Also define _ARCH_PPC64 for 32-bit on AIX.
+  } else {

Merge `else` with single `if`-statement as the sole contents of the block into 
`else if`. Comment can be placed at the top of the block controlled by the 
`else if` (and, in that position, should be indented).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107244

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


[PATCH] D107244: [AIX] Define _ARCH_PPC64 macro for 32-bit

2021-08-04 Thread Jake Egan via Phabricator via cfe-commits
Jake-Egan updated this revision to Diff 364330.
Jake-Egan added a comment.

Add comment. clang-format suggests a second indent, but I think this is correct 
because the comment is addressing the else clause.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107244

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/Preprocessor/init-ppc.c


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -391,6 +391,7 @@
 // PPC-AIX-NOT:#define __64BIT__ 1
 // PPC-AIX:#define _AIX 1
 // PPC-AIX:#define _ARCH_PPC 1
+// PPC-AIX:#define _ARCH_PPC64 1
 // PPC-AIX:#define _BIG_ENDIAN 1
 // PPC-AIX:#define _IBMR2 1
 // PPC-AIX:#define _LONG_LONG 1
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -256,6 +256,10 @@
 Builder.defineMacro("__powerpc64__");
 Builder.defineMacro("__ppc64__");
 Builder.defineMacro("__PPC64__");
+  // Also define _ARCH_PPC64 for 32-bit on AIX.
+  } else {
+if (getTriple().isOSAIX())
+  Builder.defineMacro("_ARCH_PPC64");
   }
 
   // Target properties.


Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -391,6 +391,7 @@
 // PPC-AIX-NOT:#define __64BIT__ 1
 // PPC-AIX:#define _AIX 1
 // PPC-AIX:#define _ARCH_PPC 1
+// PPC-AIX:#define _ARCH_PPC64 1
 // PPC-AIX:#define _BIG_ENDIAN 1
 // PPC-AIX:#define _IBMR2 1
 // PPC-AIX:#define _LONG_LONG 1
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -256,6 +256,10 @@
 Builder.defineMacro("__powerpc64__");
 Builder.defineMacro("__ppc64__");
 Builder.defineMacro("__PPC64__");
+  // Also define _ARCH_PPC64 for 32-bit on AIX.
+  } else {
+if (getTriple().isOSAIX())
+  Builder.defineMacro("_ARCH_PPC64");
   }
 
   // Target properties.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107522: [PowerPC][AIX] attribute aligned cannot decrease align of a vector var.

2021-08-04 Thread Sean Fertile via Phabricator via cfe-commits
sfertile created this revision.
sfertile added reviewers: stevewan, Jake-Egan, hubert.reinterpretcast.
Herald added subscribers: shchenz, nemanjai.
Herald added a reviewer: aaron.ballman.
sfertile requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

On AIX an aligned attribute cannot decrease the alignment of a variable when 
placed on a variable declaration of vector type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107522

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-vector-attr-aligned.c
  clang/test/Sema/aix-attr-aligned-vector-typedef.c


Index: clang/test/Sema/aix-attr-aligned-vector-typedef.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-vector-typedef.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -target-feature +altivec 
-target-cpu pwr7 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec 
-target-cpu pwr7 -verify -fsyntax-only %s
+
+int escape(vector int*);
+
+typedef vector int __attribute__((aligned(8))) UnderAlignedVI;
+UnderAlignedVI TypedefedGlobal;
+
+int localTypedefed(void) {
+  UnderAlignedVI TypedefedLocal;
+  return escape(); // expected-warning {{passing 8-byte aligned 
argument to 16-byte aligned parameter 1 of 'escape' may result in an unaligned 
pointer access}}
+}
Index: clang/test/CodeGen/aix-vector-attr-aligned.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vector-attr-aligned.c
@@ -0,0 +1,33 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec 
-target-cpu pwr7 -emit-llvm -o - %s | \
+// RUN:   FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -target-feature +altivec 
-target-cpu pwr7 -emit-llvm -o - %s | \
+// RUN:   FileCheck %s
+
+typedef vector int __attribute__((aligned(8))) UnderAlignedVI;
+
+vector int g32 __attribute__((aligned(32)));
+vector int g8 __attribute__((aligned(8)));
+UnderAlignedVI TypedefedGlobal;
+
+int escape(vector int*);
+
+int local32(void) {
+  vector int l32 __attribute__((aligned(32)));
+  return escape();
+}
+
+int local8(void) {
+  vector int l8 __attribute__((aligned(8)));
+  return escape();
+}
+
+// CHECK: @g32 = global <4 x i32> zeroinitializer, align 32
+// CHECK: @g8 = global <4 x i32> zeroinitializer, align 16
+// CHECK: @TypedefedGlobal = global <4 x i32> zeroinitializer, align 8
+
+// CHECK-LABEL: @local32
+// CHECK: %l32 = alloca <4 x i32>, align 32
+//
+// CHECK-LABEL: @local8
+// CHECK: %l8 = alloca <4 x i32>, align 16
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4060,11 +4060,11 @@
 return;
   }
 
+  const auto *VD = dyn_cast(D);
   if (Context.getTargetInfo().isTLSSupported()) {
 unsigned MaxTLSAlign =
 Context.toCharUnitsFromBits(Context.getTargetInfo().getMaxTLSAlign())
 .getQuantity();
-const auto *VD = dyn_cast(D);
 if (MaxTLSAlign && AlignVal > MaxTLSAlign && VD &&
 VD->getTLSKind() != VarDecl::TLS_None) {
   Diag(VD->getLocation(), diag::err_tls_var_aligned_over_maximum)
@@ -4073,6 +4073,14 @@
 }
   }
 
+  // On AIX, an aligned attribute can not decrease the alignemnt when applied
+  // to a variable declaration with vector type.
+  if (VD && Context.getTargetInfo().getTriple().isOSAIX()) {
+const Type *Ty = VD->getType().getTypePtr();
+if (Ty->isVectorType() && AlignVal < 16)
+  return;
+  }
+
   AlignedAttr *AA = ::new (Context) AlignedAttr(Context, CI, true, ICE.get());
   AA->setPackExpansion(IsPackExpansion);
   D->addAttr(AA);


Index: clang/test/Sema/aix-attr-aligned-vector-typedef.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-vector-typedef.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -target-feature +altivec -target-cpu pwr7 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec -target-cpu pwr7 -verify -fsyntax-only %s
+
+int escape(vector int*);
+
+typedef vector int __attribute__((aligned(8))) UnderAlignedVI;
+UnderAlignedVI TypedefedGlobal;
+
+int localTypedefed(void) {
+  UnderAlignedVI TypedefedLocal;
+  return escape(); // expected-warning {{passing 8-byte aligned argument to 16-byte aligned parameter 1 of 'escape' may result in an unaligned pointer access}}
+}
Index: clang/test/CodeGen/aix-vector-attr-aligned.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vector-attr-aligned.c
@@ -0,0 +1,33 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec -target-cpu pwr7 

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

2021-08-04 Thread Sean Fertile via Phabricator via cfe-commits
sfertile added a comment.

Thanks Steven, LGTM.




Comment at: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp:11
+
+// CHECK: @c = global %struct.C zeroinitializer, align 2

stevewan wrote:
> sfertile wrote:
> > Minor nit: I think the other test can live in this file, highlighting the 
> > difference between the 2 cases is nice. Also I think we should add a 
> > typedef test in here as well.
> Merged the tests as suggested. We have the typedef coverage in 
> `aix-power-alignment-typedef.cpp` and `aix-power-alignment-typedef2.cpp`, not 
> sure if want to merge those as well.
No need to merge those as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

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


[PATCH] D107433: [RISCV] Half-precision for vget/vset.

2021-08-04 Thread Zakk Chen via Phabricator via cfe-commits
khchen added inline comments.



Comment at: clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c:554
+//
+vfloat16m1_t test_vget_v_f16m2_f16m1 (vfloat16m2_t src, size_t index) {
+  return vget_v_f16m2_f16m1(src, 0);

index is an unused argument.



Comment at: clang/test/CodeGen/RISCV/rvv-intrinsics/vset.c:554
+//
+vfloat16m2_t test_vset_v_f16m1_f16m2 (vfloat16m2_t dest, size_t index, 
vfloat16m1_t val) {
+  return vset_v_f16m1_f16m2(dest, 0, val);

index is an unused argument.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107433

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


[PATCH] D107294: [clang-tidy] adds warning to suggest users replace symbols with words

2021-08-04 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp:133-134
 "readability-uppercase-literal-suffix");
+CheckFactories.registerCheck(
+"readability-use-alternative-tokens");
 CheckFactories.registerCheck(

aaron.ballman wrote:
> I think this might be a case where we want the check to either recommend 
> using alternative tokens or recommend against using alternative tokens via a 
> configuration option (so users can control readability in either direction). 
> If you agree that's a reasonable design, then I'd recommend we name this 
> `readability-alternative-tokens` to be a bit more generic. (Note, we can 
> always do the "don't use alternative tokens" implementation in a follow-up 
> patch if you don't want to do that work immediately.)
Hrmm I'll do the rename now, but this might be better off as a later patch. 
I'd rather focus on getting what I have right (along with my teased extensions) 
and then work on the opposite direction. That will (probably) be an easy 
slip-in.



Comment at: 
clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.cpp:41-42
+  Finder->addMatcher(
+  binaryOperation(hasAnyOperatorName("&&", "||", "!", "&", "|", "~", "^"))
+  .bind("operator"),
+  this);

Quuxplusone wrote:
> `~` and `!` are not binary operations, right?
This confused me too. `binaryOperation` also takes into account all overloaded 
ops.



Comment at: 
clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.cpp:53-54
+  bool IsLogicalNot = Op.getOpcode() == UnaryOperator::Opcode::UO_LNot;
+  auto Hint = FixItHint::CreateReplacement(Op.getOperatorLoc(),
+   IsLogicalNot ? "not " : "compl ");
+

aaron.ballman wrote:
> For C code, you could augment this FixIt with an include inserter, e.g., 
> https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp#L112
Thanks! This was one of the blockers for getting me C support.



Comment at: 
clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.h:25
+  bool isLanguageVersionSupported(const LangOptions ) const override {
+return LangOpts.CPlusPlus;
+  }

aaron.ballman wrote:
> We can support C for this as well, can't we? iso646.h has been around since 
> C99.
I was having difficulty in my C test for this, wimped out, and put it into the 
"I'll do it later" basket. Lemme double check that's the case?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst:6-7
+
+Finds uses of symbol-based logical and bitwise operators and recommends using
+alternative tokens instead.
+

Quuxplusone wrote:
> I strongly recommend splitting this check in two parts:
> - Use `and`, `or`, `not` for logical operations.
> - Use `bitand`, `bitor`, `xor`, `compl` for non-logical (bitwise) operations.
> The reason I recommend this is that I have //seen// people (never in a real 
> codebase, mind you, but at least bloggers and such) recommend 
> `and`/`or`/`not` for logical operations; this is a long-standing convention 
> in other languages such as Python and Perl.  Whereas I have //never// seen 
> anyone recommend e.g. `(x bitand compl mask)` over `(x & ~mask)`. So coupling 
> the two ideas together seems counterproductive, because //even if// someone 
> wanted to use the Python style in their codebase, they wouldn't be able to 
> enforce it using this check, because this check would be full of false 
> positives related to the bitwise operators.
> The current check's recommendation to replace `(a || b) => (a or b)`, `(a ^ 
> b) => (a xor b)` is particularly pernicious to non-language-lawyers, who 
> might assume that `or` and `xor` represented the same "part of speech" — 
> either both bitwise or both logical. I think this is a major motivation for 
> the Pythonic style: use English for logical `and or not`, and use symbols for 
> mathematical `& | ^ ~ << >> &= |= ~= <<= >>=`.
As agreed with Aaron above, I'll be making this configurable at a later date.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-use-alternative-tokens.rst:49-52
+Program composition
+---
+
+This check doesn't yet account for program composition. This means that the

Quuxplusone wrote:
> "Program composition" is not a term of art (except in the extremely general 
> sense of composing programs, i.e. programming). I recommend retitling this 
> section "Use of | as a pipe" or "Use of | with C++20 Ranges".
> 
> It might be worth adding a brief mention that the same is true of any 
> minigame/DSL embedded within C++; for example, this clang-tidy check will 
> also recommend changes like
> ```
> qi::rule(),
> 

[PATCH] D107294: [clang-tidy] adds warning to suggest users replace symbols with words

2021-08-04 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 364307.
cjdb marked 5 inline comments as done.
cjdb edited the summary of this revision.
cjdb added a comment.

- renames check to 'readability-alternative-tokens'
- adds section in documentation about configurability
- adds support for C source
- corrects whitespace handling in fixits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107294

Files:
  clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp
  clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.h
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/clang-tidy/checks/readability-alternative-tokens.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-alternative-tokens-no-warn.c
  clang-tools-extra/test/clang-tidy/checkers/readability-alternative-tokens.c
  clang-tools-extra/test/clang-tidy/checkers/readability-alternative-tokens.cpp
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/readability/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/readability/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/readability/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/readability/BUILD.gn
@@ -12,6 +12,7 @@
 "//llvm/lib/Support",
   ]
   sources = [
+"AlternativeTokensCheck.cpp",
 "AvoidConstParamsInDecls.cpp",
 "BracesAroundStatementsCheck.cpp",
 "ConstReturnTypeCheck.cpp",
Index: clang-tools-extra/test/clang-tidy/checkers/readability-alternative-tokens.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-alternative-tokens.cpp
@@ -0,0 +1,244 @@
+// RUN: %check_clang_tidy %s readability-alternative-tokens %t
+
+// A note to the reader: the whitespace in this file is important: `true&`
+// is lexically three tokens, but `trueandfalse` is lexed as a single
+// identifier. This test needs to make sure that the fixit applies whitespace in
+// its change.
+
+// clang-format off
+
+void logical_and()
+{
+  (void)(1&&0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'and' for logical conjunctions
+  // CHECK-FIXES: 1 and 0
+
+  (void)(1&& 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'and' for logical conjunctions
+  // CHECK-FIXES: 1 and 0
+
+  (void)(1 &&0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'and' for logical conjunctions
+  // CHECK-FIXES: 1 and 0
+
+  (void)(1 && 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'and' for logical conjunctions
+  // CHECK-FIXES: 1 and 0
+}
+
+void bitwise_and()
+{
+  (void)(0&1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'bitand' for bitwise conjunctions
+  // CHECK-FIXES: 0 bitand 1
+
+  (void)(0 &1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'bitand' for bitwise conjunctions
+  // CHECK-FIXES: 0 bitand 1
+
+  (void)(0& 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'bitand' for bitwise conjunctions
+  // CHECK-FIXES: 0 bitand 1
+
+  (void)(0 & 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'bitand' for bitwise conjunctions
+  // CHECK-FIXES: 0 bitand 1
+}
+
+void bitwise_or() {
+  (void)(0|1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'bitor' for bitwise disjunctions
+  // CHECK-FIXES: 0 bitor 1
+
+  (void)(0| 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'bitor' for bitwise disjunctions
+  // CHECK-FIXES: 0 bitor 1
+
+  (void)(0 |1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'bitor' for bitwise disjunctions
+  // CHECK-FIXES: 0 bitor 1
+
+  (void)(0 | 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'bitor' for bitwise disjunctions
+  // CHECK-FIXES: 0 bitor 1
+}
+
+void bitwise_not() {
+  (void)(~0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'compl' for bitwise negation
+  // CHECK-FIXES: compl 0
+
+  (void)(~ 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'compl' for bitwise negation
+  // CHECK-FIXES: compl 0
+}
+
+void logical_not() {
+  (void)(!0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'not' for logical negation
+  // CHECK-FIXES: not 0
+
+  (void)(! 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'not' for logical negation
+  // CHECK-FIXES: not 0
+}
+
+void logical_or() {
+  (void)(1||0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'or' for logical disjunctions
+  // CHECK-FIXES: 1 or 0
+
+  (void)(1|| 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'or' for logical disjunctions
+  // CHECK-FIXES: 1 or 0
+
+  (void)(1 ||0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'or' for logical disjunctions
+  // CHECK-FIXES: 1 or 0
+
+  (void)(1 || 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'or' for logical disjunctions
+  // CHECK-FIXES: 1 or 0
+}

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

2021-08-04 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment.

Do you need help merging this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99732

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


[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

2021-08-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Created release/13.x merge request: https://bugs.llvm.org/show_bug.cgi?id=51350
The release manager makes the call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107393

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


[PATCH] D107125: [Diagnostic] Split 'qualifier on reference type has no effect' out into a new flag

2021-08-04 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery updated this revision to Diff 364289.
lunasorcery added a comment.

Ah, that's much cleaner! I hadn't realized the test framework could do that.
I've updated the diff accordingly - your example works perfectly, so I've taken 
that and ditched the now-unnecessary other test.


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

https://reviews.llvm.org/D107125

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaCXX/ignored-reference-qualifiers-disabled.cpp


Index: clang/test/SemaCXX/ignored-reference-qualifiers-disabled.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ignored-reference-qualifiers-disabled.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -std=c++11 -Wignored-qualifiers 
-Wno-ignored-reference-qualifiers -verify=both
+// RUN: %clang_cc1 %s -std=c++11 -Wignored-qualifiers -verify=both,qual
+
+const int scalar_c(); // both-warning{{'const' type qualifier on return type 
has no effect}}
+volatile int scalar_v(); // both-warning{{'volatile' type qualifier on return 
type has no effect}}
+const volatile int scalar_cv(); // both-warning{{'const volatile' type 
qualifiers on return type have no effect}}
+
+typedef int& IntRef;
+
+const IntRef ref_c(); // qual-warning{{'const' qualifier on reference type 
'IntRef' (aka 'int &') has no effect}}
+volatile IntRef ref_v(); // qual-warning{{'volatile' qualifier on reference 
type 'IntRef' (aka 'int &') has no effect}}
+const volatile IntRef ref_cv(); // qual-warning{{'const' qualifier on 
reference type 'IntRef' (aka 'int &') has no effect}} \
+qual-warning{{'volatile' qualifier on 
reference type 'IntRef' (aka 'int &') has no effect}}
+
+template
+class container {
+   using value_type = T;
+   using reference  = value_type&;
+   reference get();
+   const reference get() const; // qual-warning{{'const' qualifier on 
reference type 'container::reference' (aka 'T &') has no effect}}
+};
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5733,7 +5733,7 @@
   "'%0' qualifier on function type %1 has unspecified behavior">;
 def warn_typecheck_reference_qualifiers : Warning<
   "'%0' qualifier on reference type %1 has no effect">,
-  InGroup;
+  InGroup;
 def err_typecheck_invalid_restrict_not_pointer : Error<
   "restrict requires a pointer or reference (%0 is invalid)">;
 def err_typecheck_invalid_restrict_not_pointer_noarg : Error<
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -400,7 +400,8 @@
 def InfiniteRecursion : DiagGroup<"infinite-recursion">;
 def PureVirtualCallFromCtorDtor: 
DiagGroup<"call-to-pure-virtual-from-ctor-dtor">;
 def GNUImaginaryConstant : DiagGroup<"gnu-imaginary-constant">;
-def IgnoredQualifiers : DiagGroup<"ignored-qualifiers">;
+def IgnoredReferenceQualifiers : DiagGroup<"ignored-reference-qualifiers">;
+def IgnoredQualifiers : DiagGroup<"ignored-qualifiers", 
[IgnoredReferenceQualifiers]>;
 def : DiagGroup<"import">;
 def GNUIncludeNext : DiagGroup<"gnu-include-next">;
 def IncompatibleMSStruct : DiagGroup<"incompatible-ms-struct">;


Index: clang/test/SemaCXX/ignored-reference-qualifiers-disabled.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ignored-reference-qualifiers-disabled.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -std=c++11 -Wignored-qualifiers -Wno-ignored-reference-qualifiers -verify=both
+// RUN: %clang_cc1 %s -std=c++11 -Wignored-qualifiers -verify=both,qual
+
+const int scalar_c(); // both-warning{{'const' type qualifier on return type has no effect}}
+volatile int scalar_v(); // both-warning{{'volatile' type qualifier on return type has no effect}}
+const volatile int scalar_cv(); // both-warning{{'const volatile' type qualifiers on return type have no effect}}
+
+typedef int& IntRef;
+
+const IntRef ref_c(); // qual-warning{{'const' qualifier on reference type 'IntRef' (aka 'int &') has no effect}}
+volatile IntRef ref_v(); // qual-warning{{'volatile' qualifier on reference type 'IntRef' (aka 'int &') has no effect}}
+const volatile IntRef ref_cv(); // qual-warning{{'const' qualifier on reference type 'IntRef' (aka 'int &') has no effect}} \
+qual-warning{{'volatile' qualifier on reference type 'IntRef' (aka 'int &') has no effect}}
+
+template
+class container {
+	using value_type = T;
+	using reference  = value_type&;
+	reference get();
+	const reference get() const; // qual-warning{{'const' qualifier on reference type 'container::reference' (aka 'T &') has no 

[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

2021-08-04 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7df405e079c5: Apply -fmacro-prefix-map to __builtin_FILE() 
(authored by Svenny, committed by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107393

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/AST/Expr.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp

Index: clang/test/CodeGenCXX/builtin-source-location.cpp
===
--- clang/test/CodeGenCXX/builtin-source-location.cpp
+++ clang/test/CodeGenCXX/builtin-source-location.cpp
@@ -1,5 +1,13 @@
 // RUN: %clang_cc1 -std=c++2a -fblocks %s -triple x86_64-unknown-unknown -emit-llvm -o %t.ll
 
+// This needs to be performed before #line directives which alter filename
+// RUN: %clang_cc1 -fmacro-prefix-map=%p=/UNLIKELY/PATH -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-PREFIX-MAP
+//
+// CHECK-PREFIX-MAP: /UNLIKELY/PATH{{/|}}builtin-source-location.cpp
+void testRemap() {
+  const char *file = __builtin_FILE();
+}
+
 #line 8 "builtin-source-location.cpp"
 
 struct source_location {
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1460,15 +1460,6 @@
   return TI.getTriple().getEnvironment() == Env.getEnvironment();
 }
 
-static void remapMacroPath(
-SmallString<256> ,
-const std::map>
-) {
-  for (const auto  : MacroPrefixMap)
-if (llvm::sys::path::replace_path_prefix(Path, Entry.first, Entry.second))
-  break;
-}
-
 /// ExpandBuiltinMacro - If an identifier token is read that is to be expanded
 /// as a builtin macro, handle it and return the next token as 'Tok'.
 void Preprocessor::ExpandBuiltinMacro(Token ) {
@@ -1550,7 +1541,7 @@
   } else {
 FN += PLoc.getFilename();
   }
-  remapMacroPath(FN, PPOpts->MacroPrefixMap);
+  getLangOpts().remapPathPrefix(FN);
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3526,6 +3526,9 @@
 GenerateArg(Args, OPT_fexperimental_relative_cxx_abi_vtables, SA);
   else
 GenerateArg(Args, OPT_fno_experimental_relative_cxx_abi_vtables, SA);
+
+  for (const auto  : Opts.MacroPrefixMap)
+GenerateArg(Args, OPT_fmacro_prefix_map_EQ, MP.first + "=" + MP.second, SA);
 }
 
 bool CompilerInvocation::ParseLangArgs(LangOptions , ArgList ,
@@ -4036,6 +4039,12 @@
options::OPT_fno_experimental_relative_cxx_abi_vtables,
TargetCXXABI::usesRelativeVTables(T));
 
+  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ)) {
+auto Split = StringRef(A).split('=');
+Opts.MacroPrefixMap.insert(
+{std::string(Split.first), std::string(Split.second)});
+  }
+
   return Diags.getNumErrors() == NumErrorsBefore;
 }
 
@@ -4108,9 +4117,6 @@
   for (const auto  : Opts.DeserializedPCHDeclsToErrorOn)
 GenerateArg(Args, OPT_error_on_deserialized_pch_decl, D, SA);
 
-  for (const auto  : Opts.MacroPrefixMap)
-GenerateArg(Args, OPT_fmacro_prefix_map_EQ, MP.first + "=" + MP.second, SA);
-
   if (Opts.PrecompiledPreambleBytes != std::make_pair(0u, false))
 GenerateArg(Args, OPT_preamble_bytes_EQ,
 Twine(Opts.PrecompiledPreambleBytes.first) + "," +
@@ -4179,12 +4185,6 @@
   for (const auto *A : Args.filtered(OPT_error_on_deserialized_pch_decl))
 Opts.DeserializedPCHDeclsToErrorOn.insert(A->getValue());
 
-  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ)) {
-auto Split = StringRef(A).split('=');
-Opts.MacroPrefixMap.insert(
-{std::string(Split.first), std::string(Split.second)});
-  }
-
   if (const Arg *A = Args.getLastArg(OPT_preamble_bytes_EQ)) {
 StringRef Value(A->getValue());
 size_t Comma = Value.find(',');
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -186,7 +186,7 @@
   !getModule().getSourceFileName().empty()) {
 std::string Path = getModule().getSourceFileName();
 // Check if a path substitution is needed from the MacroPrefixMap.
-for (const auto  : PPO.MacroPrefixMap)
+for (const auto  : LangOpts.MacroPrefixMap)
   if (Path.rfind(Entry.first, 0) != std::string::npos) {
 Path = Entry.second + 

[clang] 7df405e - Apply -fmacro-prefix-map to __builtin_FILE()

2021-08-04 Thread Fangrui Song via cfe-commits

Author: Pavel Asyutchenko
Date: 2021-08-04T16:42:14-07:00
New Revision: 7df405e079c5045562c53f7a2504b85f423078be

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

LOG: Apply -fmacro-prefix-map to __builtin_FILE()

This matches the behavior of GCC.
Patch does not change remapping logic itself, so adding one simple smoke test 
should be enough.

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/Options.td
clang/include/clang/Lex/PreprocessorOptions.h
clang/lib/AST/Expr.cpp
clang/lib/Basic/LangOptions.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Lex/PPMacroExpansion.cpp
clang/test/CodeGenCXX/builtin-source-location.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index 97766e059c040..72793114dbb38 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -367,6 +367,9 @@ class LangOptions : public LangOptionsBase {
   /// A list of all -fno-builtin-* function names (e.g., memset).
   std::vector NoBuiltinFuncs;
 
+  /// A prefix map for __FILE__, __BASE_FILE__ and __builtin_FILE().
+  std::map> MacroPrefixMap;
+
   /// Triples of the OpenMP targets that the host code codegen should
   /// take into account in order to generate accurate offloading descriptors.
   std::vector OMPTargetTriples;
@@ -473,6 +476,9 @@ class LangOptions : public LangOptionsBase {
   }
 
   bool isSYCL() const { return SYCLIsDevice || SYCLIsHost; }
+
+  /// Remap path prefix according to -fmacro-prefix-path option.
+  void remapPathPrefix(SmallString<256> ) const;
 };
 
 /// Floating point control options

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 712aa9f8c5c9c..3715172917204 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2840,10 +2840,10 @@ def fcoverage_prefix_map_EQ
 HelpText<"remap file source paths in coverage mapping">;
 def ffile_prefix_map_EQ
   : Joined<["-"], "ffile-prefix-map=">, Group,
-HelpText<"remap file source paths in debug info and predefined 
preprocessor macros">;
+HelpText<"remap file source paths in debug info, predefined preprocessor 
macros and __builtin_FILE()">;
 def fmacro_prefix_map_EQ
-  : Joined<["-"], "fmacro-prefix-map=">, Group, 
Flags<[CC1Option]>,
-HelpText<"remap file source paths in predefined preprocessor macros">;
+  : Joined<["-"], "fmacro-prefix-map=">, Group, Flags<[CC1Option]>,
+HelpText<"remap file source paths in predefined preprocessor macros and 
__builtin_FILE()">;
 defm force_dwarf_frame : BoolFOption<"force-dwarf-frame",
   CodeGenOpts<"ForceDwarfFrameSection">, DefaultFalse,
   PosFlag, 
NegFlag>;

diff  --git a/clang/include/clang/Lex/PreprocessorOptions.h 
b/clang/include/clang/Lex/PreprocessorOptions.h
index 4ad7305e3b39a..eea0c1a2986e2 100644
--- a/clang/include/clang/Lex/PreprocessorOptions.h
+++ b/clang/include/clang/Lex/PreprocessorOptions.h
@@ -202,9 +202,6 @@ class PreprocessorOptions {
   /// build it again.
   std::shared_ptr FailedModules;
 
-  /// A prefix map for __FILE__ and __BASE_FILE__.
-  std::map> MacroPrefixMap;
-
   /// Contains the currently active skipped range mappings for skipping 
excluded
   /// conditional directives.
   ///

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index e8b4aaa2b81e6..11f10d4695fcd 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -2233,8 +2233,11 @@ APValue SourceLocExpr::EvaluateInContext(const 
ASTContext ,
   };
 
   switch (getIdentKind()) {
-  case SourceLocExpr::File:
-return MakeStringLiteral(PLoc.getFilename());
+  case SourceLocExpr::File: {
+SmallString<256> Path(PLoc.getFilename());
+Ctx.getLangOpts().remapPathPrefix(Path);
+return MakeStringLiteral(Path);
+  }
   case SourceLocExpr::Function: {
 const Decl *CurDecl = dyn_cast_or_null(Context);
 return MakeStringLiteral(

diff  --git a/clang/lib/Basic/LangOptions.cpp b/clang/lib/Basic/LangOptions.cpp
index dc392d5352aae..bebf3178426f0 100644
--- a/clang/lib/Basic/LangOptions.cpp
+++ b/clang/lib/Basic/LangOptions.cpp
@@ -11,6 +11,8 @@
 
//===--===//
 
 #include "clang/Basic/LangOptions.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Path.h"
 
 using namespace clang;
 
@@ -48,6 +50,12 @@ VersionTuple LangOptions::getOpenCLVersionTuple() const {
   return VersionTuple(Ver / 100, (Ver % 100) / 10);
 }
 
+void LangOptions::remapPathPrefix(SmallString<256> ) const {
+  for (const 

[PATCH] D106799: [OpenMP] Always inline the OpenMP outlined function

2021-08-04 Thread Mike Rice via Phabricator via cfe-commits
mikerice added a comment.

Looks like now omp.outlined functions are marked with both 'noinline' and 
'alwaysinline'.  See https://bugs.llvm.org/show_bug.cgi?id=51349


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106799

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


[PATCH] D107503: [clang][Rewriter] patch to fix bug with ReplaceText erasing too many characters when text has been inserted before the replace location.

2021-08-04 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon updated this revision to Diff 364283.
ajohnson-uoregon added a comment.

making patch happy???


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107503

Files:
  clang/include/clang/Rewrite/Core/Rewriter.h
  clang/lib/Rewrite/Rewriter.cpp
  clang/unittests/Rewrite/RewriterTest.cpp

Index: clang/unittests/Rewrite/RewriterTest.cpp
===
--- clang/unittests/Rewrite/RewriterTest.cpp
+++ clang/unittests/Rewrite/RewriterTest.cpp
@@ -62,6 +62,7 @@
   // Check that correct text is replaced for each range type.  Ranges remain in
   // terms of the original text but include the new text.
   StringRef Code = "int main(int argc, char *argv[]) { return argc; }";
+  //  foogc; }
   //replace char range with "foo" ^~
   //  get ^ = "foogc;"
   //   replace token range with "bar" ^~++
@@ -77,4 +78,54 @@
   EXPECT_EQ(T.Rewrite.getRewrittenText(T.makeCharRange(42, 47)), "0;");
 }
 
+TEST(Rewriter, ReplaceAfterInsertNoOpts) {
+  // Check that remove/insert after insert behaves correctly based on the
+  // RewriteOptions passed. This should erase extra text.
+
+  StringRef code = "int main(int argc, char *argv[]) { return argc; } double "
+   "f(int text_to_erase) {}";
+  //insert "int answer = 42;" ^
+  //  replace with 42 ^
+  // extra text to erase ^
+  std::unique_ptr AST = tooling::buildASTFromCode(code);
+  ASTContext  = AST->getASTContext();
+  Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());
+  SourceLocation FileStart = AST->getStartOfMainFileID();
+  SourceLocation insert_point = FileStart.getLocWithOffset(34);
+  Rewrite.InsertText(insert_point, "int answer = 42;");
+  SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(34),
+  FileStart.getLocWithOffset(45));
+  Rewrite.ReplaceText(replace_range, "42;");
+  EXPECT_EQ(
+  Rewrite.getRewrittenText(SourceRange(FileStart.getLocWithOffset(33),
+   FileStart.getLocWithOffset(65))),
+  "{int answer = 42;42; text_to_erase");
+}
+
+TEST(Rewriter, ReplaceAfterInsertOpts) {
+  // Check that remove/insert after insert behaves correctly based on the
+  // RewriteOptions passed. This should NOT erase extra text.
+
+  StringRef code = "int main(int argc, char *argv[]) { return argc; } double "
+   "f(int text_to_erase) {}";
+  //insert "int answer = 42;" ^
+  //  replace with 42 ^
+  // extra text to erase ^
+  std::unique_ptr AST = tooling::buildASTFromCode(code);
+  ASTContext  = AST->getASTContext();
+  Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());
+  SourceLocation FileStart = AST->getStartOfMainFileID();
+  SourceLocation insert_point = FileStart.getLocWithOffset(34);
+  Rewrite.InsertText(insert_point, "int answer = 42;");
+  SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(34),
+  FileStart.getLocWithOffset(46));
+  Rewriter::RewriteOptions opts;
+  opts.IncludeInsertsAtBeginOfRange = false;
+  Rewrite.ReplaceText(replace_range, "42;", opts);
+  EXPECT_EQ(
+  Rewrite.getRewrittenText(SourceRange(FileStart.getLocWithOffset(33),
+   FileStart.getLocWithOffset(58))),
+  "{int answer = 42;42; } double f(");
+}
+
 } // anonymous namespace
Index: clang/lib/Rewrite/Rewriter.cpp
===
--- clang/lib/Rewrite/Rewriter.cpp
+++ clang/lib/Rewrite/Rewriter.cpp
@@ -327,13 +327,14 @@
   return false;
 }
 
-bool Rewriter::ReplaceText(SourceRange range, SourceRange replacementRange) {
+bool Rewriter::ReplaceText(SourceRange range, SourceRange replacementRange,
+   RewriteOptions opts) {
   if (!isRewritable(range.getBegin())) return true;
   if (!isRewritable(range.getEnd())) return true;
   if (replacementRange.isInvalid()) return true;
   SourceLocation start = range.getBegin();
-  unsigned origLength = getRangeSize(range);
-  unsigned newLength = getRangeSize(replacementRange);
+  unsigned origLength = getRangeSize(range, opts);
+  unsigned newLength = getRangeSize(replacementRange, opts);
   FileID FID;
   unsigned newOffs = getLocationOffsetAndFileID(replacementRange.getBegin(),
 FID);
Index: clang/include/clang/Rewrite/Core/Rewriter.h
===
--- clang/include/clang/Rewrite/Core/Rewriter.h
+++ 

[PATCH] D107503: [clang][Rewriter] patch to fix bug with ReplaceText erasing too many characters when text has been inserted before the replace location.

2021-08-04 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon updated this revision to Diff 364282.
ajohnson-uoregon retitled this revision from "[clang][Rewriter] patch to fix 
bug with ReplaceText erasing too many characters when text has been inserted 
before the replace location. " to "[clang][Rewriter] patch to fix bug with 
ReplaceText erasing too many characters when text has been inserted before the 
replace location.".
ajohnson-uoregon added a comment.

Making clang-format happy


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107503

Files:
  clang/include/clang/Rewrite/Core/Rewriter.h
  clang/lib/Rewrite/Rewriter.cpp
  clang/unittests/Rewrite/RewriterTest.cpp

Index: clang/unittests/Rewrite/RewriterTest.cpp
===
--- clang/unittests/Rewrite/RewriterTest.cpp
+++ clang/unittests/Rewrite/RewriterTest.cpp
@@ -78,34 +78,54 @@
   EXPECT_EQ(T.Rewrite.getRewrittenText(T.makeCharRange(42, 47)), "0;");
 }
 
-TEST(Rewriter, ReplaceAfterInsert) {
+TEST(Rewriter, ReplaceAfterInsertNoOpts) {
   // Check that remove/insert after insert behaves correctly based on the
-  // RewriteOptions passed.
+  // RewriteOptions passed. This should erase extra text.
 
-  StringRef code = "int main(int argc, char *argv[]) { return argc; } double f(int text_to_erase) {}";
-  //  ^ insert "int answer = 42;"
-  //  ^ replace with answer
-  //  ^ replace with 42
-  //   extra text to erase ^
+  StringRef code = "int main(int argc, char *argv[]) { return argc; } double "
+   "f(int text_to_erase) {}";
+  //insert "int answer = 42;" ^
+  //  replace with 42 ^
+  // extra text to erase ^
   std::unique_ptr AST = tooling::buildASTFromCode(code);
   ASTContext  = AST->getASTContext();
-  Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());;
+  Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());
   SourceLocation FileStart = AST->getStartOfMainFileID();
   SourceLocation insert_point = FileStart.getLocWithOffset(34);
   Rewrite.InsertText(insert_point, "int answer = 42;");
-  SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(42),
+  SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(34),
   FileStart.getLocWithOffset(45));
+  Rewrite.ReplaceText(replace_range, "42;");
+  EXPECT_EQ(
+  Rewrite.getRewrittenText(SourceRange(FileStart.getLocWithOffset(33),
+   FileStart.getLocWithOffset(65))),
+  "{int answer = 42;42; text_to_erase");
+}
+
+TEST(Rewriter, ReplaceAfterInsertOpts) {
+  // Check that remove/insert after insert behaves correctly based on the
+  // RewriteOptions passed. This should NOT erase extra text.
+
+  StringRef code = "int main(int argc, char *argv[]) { return argc; } double "
+   "f(int text_to_erase) {}";
+  //insert "int answer = 42;" ^
+  //  replace with 42 ^
+  // extra text to erase ^
+  std::unique_ptr AST = tooling::buildASTFromCode(code);
+  ASTContext  = AST->getASTContext();
+  Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());
+  SourceLocation FileStart = AST->getStartOfMainFileID();
+  SourceLocation insert_point = FileStart.getLocWithOffset(34);
+  Rewrite.InsertText(insert_point, "int answer = 42;");
+  SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(34),
+  FileStart.getLocWithOffset(46));
   Rewriter::RewriteOptions opts;
   opts.IncludeInsertsAtBeginOfRange = false;
-  Rewrite.ReplaceText(replace_range, "answer", opts);
-  EXPECT_EQ(Rewrite.getRewrittenText(
-  SourceRange(FileStart.getLocWithOffset(33), FileStart.getLocWithOffset(48))),
-"{int answer = 42; return answer; }");
-  Rewrite.ReplaceText(replace_range, "42", Rewriter::RewriteOptions());
-  EXPECT_EQ(Rewrite.getRewrittenText(
-  SourceRange(FileStart.getLocWithOffset(33), FileStart.getLocWithOffset(55))),
-"{int answer = 42; return 42; } double");
+  Rewrite.ReplaceText(replace_range, "42;", opts);
+  EXPECT_EQ(
+  Rewrite.getRewrittenText(SourceRange(FileStart.getLocWithOffset(33),
+   FileStart.getLocWithOffset(58))),
+  "{int answer = 42;42; } double f(");
 }
 
-
 } // anonymous namespace
Index: clang/lib/Rewrite/Rewriter.cpp
===
--- clang/lib/Rewrite/Rewriter.cpp
+++ clang/lib/Rewrite/Rewriter.cpp
@@ -327,7 +327,8 @@
   return false;
 }
 
-bool 

[PATCH] D106030: [Clang] add support for error+warning fn attrs

2021-08-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers marked an inline comment as done.
nickdesaulniers added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1205
 def BackendOptimizationFailure : DiagGroup<"pass-failed">;
+def BackendUserDiagnostic : DiagGroup<"user-diagnostic">;
 

aaron.ballman wrote:
> nickdesaulniers wrote:
> > aaron.ballman wrote:
> > > nickdesaulniers wrote:
> > > > aaron.ballman wrote:
> > > > > nickdesaulniers wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > GCC doesn't seem to support this spelling -- do they have a 
> > > > > > > different one we should reuse?
> > > > > > I think these diagnostics don't have corresponding flags in GCC, so 
> > > > > > they cannot be disabled.
> > > > > > 
> > > > > > Without adding this, clang/test/Misc/warning-flags.c would fail, 
> > > > > > because I was adding a new unnamed warning 
> > > > > > `warn_fe_backend_user_diagnostic`.  Perhaps I should not be adding 
> > > > > > this line here, and doing something else?
> > > > > > 
> > > > > > That test says we shouldn't be adding new warnings not controlled 
> > > > > > by flags, but that's very much my intention.
> > > > > > 
> > > > > > Though now `-Wno-user-diagnostic` doesn't produce a 
> > > > > > `-Wunknown-warning-option` diagnostic...
> > > > > Ah! I think the warning attribute should be controllable via a 
> > > > > diagnostic flag (we should always be able to disable warnings with 
> > > > > some sort of flag) and I don't think the error attribute should be 
> > > > > controllable (an error is an error and should stop translation, same 
> > > > > as with `#error`).
> > > > > 
> > > > > Normally, I'd say "let's use the same flag that controls `#warning`" 
> > > > > but...
> > > > > ```
> > > > > def PoundWarning : DiagGroup<"#warnings">;
> > > > > ```
> > > > > That's... not exactly an obvious flag for the warning attribute. So I 
> > > > > would probably name it `warning-attributes` similar to how we have 
> > > > > `deprecated-attributes` already.
> > > > Done, now `-Wno-warning-attributes` doesn't produce 
> > > > `-Wunknown-warning-option`, but also doesn't disable the diagnostic.  
> > > > Was there something else I needed to add?
> > > huh, that sounds suspicious -- you don't need to do anything special for 
> > > `-Wno-foo` handling, that should be automatically supported via tablegen. 
> > > I'm not certain why you're not seeing `-Wno-warning-attributes` silencing 
> > > the warnings.
> > Ah! Because I was testing `__attribute__((error("")))` not 
> > `__attribute__((warning("")))`. `-Wno-warning-attributes` only affects the 
> > latter, not the former.  I should add a test for `-Wno-warning-attributes`! 
> > Is this something I also need to add documentation for?
> Given that this behavior surprised you, I certainly wouldn't oppose 
> mentioning it in the documentation, but I also don't think it's strictly 
> required. That said, I do agree about adding some additional test coverage 
> for when the warning is disabled.
> 
> Just to double-check: do you think this functionality needs an "escape hatch" 
> for the error case? e.g., should the `error` attribute generate a warning 
> diagnostic that defaults to being an error, so we have 
> `-Wno-warning-attributes` to turn off `warning` attribute diagnostics and 
> `-Wno-error-attributes` to turn off `error` attribute diagnostics?
Ah, GCC also controls this via `-Wattribute-warning`; let me rename my 
implementation.

> do you think this functionality needs an "escape hatch" for the error case?

No.  If users don't want an error, then they should prefer 
`__attribute__((warning("")))` to `__attribute__((error("")))`.



Comment at: llvm/include/llvm/IR/DiagnosticInfo.h:1079
+public:
+  // TODO: make LocCookie optional
+  DiagnosticInfoDontCall(StringRef CalleeName, unsigned LocCookie)

nickdesaulniers wrote:
> TODO: play with llvm::Optional
Ok, that was fun, but I didn't find it made construction sites of 
`DiagnosticInfoDontCall` nicer.  This is also how `DiagnosticInfoInlineAsm` 
works, which is where I got the idea for `!srcloc` from.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106030

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


[PATCH] D106030: [Clang] add support for error+warning fn attrs

2021-08-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 364280.
nickdesaulniers marked 5 inline comments as done.
nickdesaulniers edited the summary of this revision.
nickdesaulniers added a comment.

- remove stale todos, change warning to -Wwarning-attributes to match GCC, 
update docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106030

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-error.c
  clang/test/CodeGen/attr-warning.c
  clang/test/Frontend/backend-attribute-error-warning-optimize.c
  clang/test/Frontend/backend-attribute-error-warning.c
  clang/test/Frontend/backend-attribute-error-warning.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Misc/serialized-diags-driver.c
  clang/test/Sema/attr-error.c
  clang/test/Sema/attr-warning.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/test/CodeGen/X86/attr-dontcall.ll
  llvm/test/ThinLTO/X86/dontcall.ll

Index: llvm/test/ThinLTO/X86/dontcall.ll
===
--- /dev/null
+++ llvm/test/ThinLTO/X86/dontcall.ll
@@ -0,0 +1,33 @@
+; RUN: split-file %s %t
+; RUN: opt -module-summary %t/a.s -o %t/a.bc
+; RUN: opt -module-summary %t/b.s -o %t/b.bc
+; RUN: not llvm-lto2 run %t/a.bc %t/b.bc -o %t/out -save-temps 2>&1 \
+; RUN:   -r=%t/a.bc,callee,px \
+; RUN:   -r=%t/b.bc,callee,x  \
+; RUN:   -r=%t/b.bc,caller,px
+
+; TODO: As part of LTO, we check that types match, but *we don't yet check that
+; attributes match!!! What should happen if we remove "dontcall" from the
+; definition or declaration of @callee?
+
+; CHECK: call to callee marked "dontcall"
+
+;--- a.s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @callee() "dontcall" noinline {
+  ret i32 42
+}
+
+;--- b.s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare i32 @callee() "dontcall"
+
+define i32 @caller() {
+entry:
+  %0 = call i32 @callee()
+  ret i32 %0
+}
Index: llvm/test/CodeGen/X86/attr-dontcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/attr-dontcall.ll
@@ -0,0 +1,11 @@
+; RUN: not llc -global-isel=0 -fast-isel=0 -stop-after=finalize-isel %s 2>&1 | FileCheck %s
+; RUN: not llc -global-isel=0 -fast-isel=1 -stop-after=finalize-isel %s 2>&1 | FileCheck %s
+; RUN: not llc -global-isel=1 -fast-isel=0 -stop-after=irtranslator %s 2>&1 | FileCheck %s
+
+declare void @foo() "dontcall"
+define void @bar() {
+  call void @foo()
+  ret void
+}
+
+; CHECK: error: call to foo marked "dontcall"
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -401,3 +401,7 @@
 
 void OptimizationRemarkAnalysisFPCommute::anchor() {}
 void OptimizationRemarkAnalysisAliasing::anchor() {}
+
+void DiagnosticInfoDontCall::print(DiagnosticPrinter ) const {
+  DP << "call to " << getFunctionName() << " marked \"dontcall\"";
+}
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -69,6 +69,7 @@
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GetElementPtrTypeIterator.h"
 #include "llvm/IR/InlineAsm.h"
@@ -7918,6 +7919,15 @@
   }
 
   if (Function *F = I.getCalledFunction()) {
+if (F->hasFnAttribute("dontcall")) {
+  unsigned LocCookie = 0;
+  if (MDNode *MD = I.getMetadata("srcloc"))
+LocCookie =
+mdconst::extract(MD->getOperand(0))->getZExtValue();
+  DiagnosticInfoDontCall D(F->getName(), LocCookie);
+  DAG.getContext()->diagnose(D);
+}
+
 if (F->isDeclaration()) {
   // Is this an LLVM intrinsic or a target-specific intrinsic?
   unsigned IID = F->getIntrinsicID();
Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp

[PATCH] D107492: [clang] Replace asm with __asm__ in cuda header

2021-08-04 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield updated this revision to Diff 364277.
JonChesterfield added a comment.

- consistent __volatile__


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107492

Files:
  clang/lib/Headers/__clang_cuda_device_functions.h

Index: clang/lib/Headers/__clang_cuda_device_functions.h
===
--- clang/lib/Headers/__clang_cuda_device_functions.h
+++ clang/lib/Headers/__clang_cuda_device_functions.h
@@ -34,10 +34,12 @@
   return __nv_brevll(__a);
 }
 #if defined(__cplusplus)
-__DEVICE__ void __brkpt() { asm volatile("brkpt;"); }
+__DEVICE__ void __brkpt() { __asm__ __volatile__("brkpt;"); }
 __DEVICE__ void __brkpt(int __a) { __brkpt(); }
 #else
-__DEVICE__ void __attribute__((overloadable)) __brkpt(void) { asm volatile("brkpt;"); }
+__DEVICE__ void __attribute__((overloadable)) __brkpt(void) {
+  __asm__ __volatile__("brkpt;");
+}
 __DEVICE__ void __attribute__((overloadable)) __brkpt(int __a) { __brkpt(); }
 #endif
 __DEVICE__ unsigned int __byte_perm(unsigned int __a, unsigned int __b,
@@ -507,7 +509,7 @@
 }
 
 // Parameter must have a known integer value.
-#define __prof_trigger(__a) asm __volatile__("pmevent \t%0;" ::"i"(__a))
+#define __prof_trigger(__a) __asm__ __volatile__("pmevent \t%0;" ::"i"(__a))
 __DEVICE__ int __rhadd(int __a, int __b) { return __nv_rhadd(__a, __b); }
 __DEVICE__ unsigned int __sad(int __a, int __b, unsigned int __c) {
   return __nv_sad(__a, __b, __c);
@@ -526,7 +528,7 @@
 __DEVICE__ void __threadfence(void) { __nvvm_membar_gl(); }
 __DEVICE__ void __threadfence_block(void) { __nvvm_membar_cta(); };
 __DEVICE__ void __threadfence_system(void) { __nvvm_membar_sys(); };
-__DEVICE__ void __trap(void) { asm volatile("trap;"); }
+__DEVICE__ void __trap(void) { __asm__ __volatile__("trap;"); }
 __DEVICE__ unsigned int __uAtomicAdd(unsigned int *__p, unsigned int __v) {
   return __nvvm_atom_add_gen_i((int *)__p, __v);
 }
@@ -1051,122 +1053,136 @@
 }
 __DEVICE__ unsigned int __vabs2(unsigned int __a) {
   unsigned int r;
-  asm("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(0), "r"(0));
+  __asm__("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabs4(unsigned int __a) {
   unsigned int r;
-  asm("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(0), "r"(0));
+  __asm__("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsdiffs2(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 
 __DEVICE__ unsigned int __vabsdiffs4(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsdiffu2(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff2.u32.u32.u32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff2.u32.u32.u32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsdiffu4(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff4.u32.u32.u32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff4.u32.u32.u32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsss2(unsigned int __a) {
   unsigned int r;
-  asm("vabsdiff2.s32.s32.s32.sat %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(0), "r"(0));
+  __asm__("vabsdiff2.s32.s32.s32.sat %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsss4(unsigned int __a) {
   unsigned int r;
-  asm("vabsdiff4.s32.s32.s32.sat %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(0), "r"(0));
+  __asm__("vabsdiff4.s32.s32.s32.sat %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vadd2(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vadd2.u32.u32.u32 %0,%1,%2,%3;" : "=r"(r) : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vadd2.u32.u32.u32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vadd4(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vadd4.u32.u32.u32 %0,%1,%2,%3;" : "=r"(r) : "r"(__a), "r"(__b), 

[PATCH] D107492: [clang] Replace asm with __asm__ in cuda header

2021-08-04 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

In D107492#2926910 , @tra wrote:

> In D107492#2926871 , 
> @JonChesterfield wrote:
>
>> Therefore I'd like to leave it as `__asm__ volatile`.
>
> Being the one who introduced inconsistent use of `__volatile__` and 
> `volatile` in this header, I'm pretty sure that PTX's notion of volatile is 
> not involved for inline asm of `trap` and `brkpt`. It should be fine changing 
> them to `__volatile__` for the sake of uniformity. 
> Up to you.

Ah, I had missed that we already had an instance of `__volatile__`. We have one 
`__volatile__` and three `volatile`. I like consistency and it seems you would 
prefer '__' to '', will patch that at the same time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107492

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


[PATCH] D107492: [clang] Replace asm with __asm__ in cuda header

2021-08-04 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D107492#2926871 , @JonChesterfield 
wrote:

> Therefore I'd like to leave it as `__asm__ volatile`.

Being the one who introduced inconsistent use of `__volatile__` and `volatile` 
in this header, I'm pretty sure that PTX's notion of volatile is not involved 
for inline asm of `trap` and `brkpt`. It should be fine changing them to 
`__volatile__` for the sake of uniformity. 
Up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107492

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


[PATCH] D107492: [clang] Replace asm with __asm__ in cuda header

2021-08-04 Thread Evgeny Mankov via Phabricator via cfe-commits
emankov added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:1043
 }
-#else // CUDA_VERSION >= 9020
+#else  // CUDA_VERSION >= 9020
 // CUDA no longer provides inline assembly (or bitcode) implementation of these

JonChesterfield wrote:
> JonChesterfield wrote:
> > emankov wrote:
> > > Unneeded formatting.
> > Correct formatting though. This is what clang-format did to the whole file. 
> > I'll revert the space in favour of git-clang-format if you prefer
> git-clang-format also wanted to insert the space here and I then had to use 
> -nolint. However hopefully that removes an async round trip in the review 
> chain. Some of the previous lines were 81 characters. I suppose we could 
> apply a whitespace-only clang-format patch first but it doesn't seem worth 
> the time.
The point is that this formatting is not related to the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107492

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


[PATCH] D107492: [clang] Replace asm with __asm__ in cuda header

2021-08-04 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:1043
 }
-#else // CUDA_VERSION >= 9020
+#else  // CUDA_VERSION >= 9020
 // CUDA no longer provides inline assembly (or bitcode) implementation of these

JonChesterfield wrote:
> emankov wrote:
> > Unneeded formatting.
> Correct formatting though. This is what clang-format did to the whole file. 
> I'll revert the space in favour of git-clang-format if you prefer
git-clang-format also wanted to insert the space here and I then had to use 
-nolint. However hopefully that removes an async round trip in the review 
chain. Some of the previous lines were 81 characters. I suppose we could apply 
a whitespace-only clang-format patch first but it doesn't seem worth the time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107492

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


[PATCH] D107492: [clang] Replace asm with __asm__ in cuda header

2021-08-04 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield updated this revision to Diff 364274.
JonChesterfield added a comment.

- whitespace change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107492

Files:
  clang/lib/Headers/__clang_cuda_device_functions.h

Index: clang/lib/Headers/__clang_cuda_device_functions.h
===
--- clang/lib/Headers/__clang_cuda_device_functions.h
+++ clang/lib/Headers/__clang_cuda_device_functions.h
@@ -34,10 +34,12 @@
   return __nv_brevll(__a);
 }
 #if defined(__cplusplus)
-__DEVICE__ void __brkpt() { asm volatile("brkpt;"); }
+__DEVICE__ void __brkpt() { __asm__ volatile("brkpt;"); }
 __DEVICE__ void __brkpt(int __a) { __brkpt(); }
 #else
-__DEVICE__ void __attribute__((overloadable)) __brkpt(void) { asm volatile("brkpt;"); }
+__DEVICE__ void __attribute__((overloadable)) __brkpt(void) {
+  __asm__ volatile("brkpt;");
+}
 __DEVICE__ void __attribute__((overloadable)) __brkpt(int __a) { __brkpt(); }
 #endif
 __DEVICE__ unsigned int __byte_perm(unsigned int __a, unsigned int __b,
@@ -507,7 +509,7 @@
 }
 
 // Parameter must have a known integer value.
-#define __prof_trigger(__a) asm __volatile__("pmevent \t%0;" ::"i"(__a))
+#define __prof_trigger(__a) __asm__ __volatile__("pmevent \t%0;" ::"i"(__a))
 __DEVICE__ int __rhadd(int __a, int __b) { return __nv_rhadd(__a, __b); }
 __DEVICE__ unsigned int __sad(int __a, int __b, unsigned int __c) {
   return __nv_sad(__a, __b, __c);
@@ -526,7 +528,7 @@
 __DEVICE__ void __threadfence(void) { __nvvm_membar_gl(); }
 __DEVICE__ void __threadfence_block(void) { __nvvm_membar_cta(); };
 __DEVICE__ void __threadfence_system(void) { __nvvm_membar_sys(); };
-__DEVICE__ void __trap(void) { asm volatile("trap;"); }
+__DEVICE__ void __trap(void) { __asm__ volatile("trap;"); }
 __DEVICE__ unsigned int __uAtomicAdd(unsigned int *__p, unsigned int __v) {
   return __nvvm_atom_add_gen_i((int *)__p, __v);
 }
@@ -1051,122 +1053,136 @@
 }
 __DEVICE__ unsigned int __vabs2(unsigned int __a) {
   unsigned int r;
-  asm("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(0), "r"(0));
+  __asm__("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabs4(unsigned int __a) {
   unsigned int r;
-  asm("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(0), "r"(0));
+  __asm__("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsdiffs2(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 
 __DEVICE__ unsigned int __vabsdiffs4(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsdiffu2(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff2.u32.u32.u32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff2.u32.u32.u32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsdiffu4(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff4.u32.u32.u32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff4.u32.u32.u32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsss2(unsigned int __a) {
   unsigned int r;
-  asm("vabsdiff2.s32.s32.s32.sat %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(0), "r"(0));
+  __asm__("vabsdiff2.s32.s32.s32.sat %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsss4(unsigned int __a) {
   unsigned int r;
-  asm("vabsdiff4.s32.s32.s32.sat %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(0), "r"(0));
+  __asm__("vabsdiff4.s32.s32.s32.sat %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vadd2(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vadd2.u32.u32.u32 %0,%1,%2,%3;" : "=r"(r) : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vadd2.u32.u32.u32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vadd4(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vadd4.u32.u32.u32 %0,%1,%2,%3;" : "=r"(r) : "r"(__a), "r"(__b), "r"(0));
+  

[PATCH] D107024: [DIBuilder] Do not replace empty enum types

2021-08-04 Thread Ellis Hoag via Phabricator via cfe-commits
ellis added a comment.

In D107024#2917202 , @aprantl wrote:

> I think that looks fine — I wonder if we should change the IR pretty printer 
> to display empty arrays inline as `elements: !{}`, too.

I think that's a good idea, but it would require changing lots of tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107024

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


[PATCH] D107492: [clang] Replace asm with __asm__ in cuda header

2021-08-04 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

Re: `__volatile__`. I've done some background reading on it but can't find the 
original motivation. `__asm__` exists because the asm word is in the 
application namespace and asm is an extension. Volatile has been with us since 
C89 though, and is also accepted under -ansi. I haven't been able to find a set 
of compiler flags that reject it.

Cuda however uses volatile to mean a different thing to C as it is part of the 
atomic model. From poking at `-x cuda` in godbolt it looks like clang lowers 
cuda volatile to IR volatile, and not to relaxed atomic. I'm not confident 
that's correct, in particular i++ on a volatile variable turns into a load and 
a store, not an atomicrmw. That's orthogonal to this particular change but does 
make me reluctant to touch the word 'volatile' in anything that is compiled as 
cuda.

Therefore I'd like to leave it as `__asm__ volatile`.




Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:37
 #if defined(__cplusplus)
-__DEVICE__ void __brkpt() { asm volatile("brkpt;"); }
+__DEVICE__ void __brkpt() { __asm__ volatile("brkpt;"); }
 __DEVICE__ void __brkpt(int __a) { __brkpt(); }

tra wrote:
> Should we also change `volatile` ->  `__volatile__` here in other places in 
> the file?
Replying at the end



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:1043
 }
-#else // CUDA_VERSION >= 9020
+#else  // CUDA_VERSION >= 9020
 // CUDA no longer provides inline assembly (or bitcode) implementation of these

emankov wrote:
> Unneeded formatting.
Correct formatting though. This is what clang-format did to the whole file. 
I'll revert the space in favour of git-clang-format if you prefer



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:1057
+  __asm__("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));

emankov wrote:
> Tabs are not allowed, please use whitespaces instead.
I've been there! It's not a tab,  that's the symbol phabricator unhelpfully 
uses to indicate whitespace change. In this case, four extra spaces to keep the 
: lined up with the brace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107492

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


[PATCH] D107497: [PowerPC][AIX] Limit attribute aligned to 4096.

2021-08-04 Thread Sean Fertile via Phabricator via cfe-commits
sfertile updated this revision to Diff 364264.
sfertile added a comment.

Fix formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107497

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/aix-attr-aligned-limit.c


Index: clang/test/Sema/aix-attr-aligned-limit.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-limit.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -fsyntax-only -verify %s
+//
+int a __attribute__((aligned(8192))); // expected-error {{requested alignment 
must be 4096 bytes or smaller}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4054,6 +4054,9 @@
   unsigned MaximumAlignment = Sema::MaximumAlignment;
   if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF())
 MaximumAlignment = std::min(MaximumAlignment, 8192u);
+  else if (Context.getTargetInfo().getTriple().isOSAIX())
+MaximumAlignment = std::min(MaximumAlignment, 4096u);
+
   if (AlignVal > MaximumAlignment) {
 Diag(AttrLoc, diag::err_attribute_aligned_too_great)
 << MaximumAlignment << E->getSourceRange();


Index: clang/test/Sema/aix-attr-aligned-limit.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-limit.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -fsyntax-only -verify %s
+//
+int a __attribute__((aligned(8192))); // expected-error {{requested alignment must be 4096 bytes or smaller}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4054,6 +4054,9 @@
   unsigned MaximumAlignment = Sema::MaximumAlignment;
   if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF())
 MaximumAlignment = std::min(MaximumAlignment, 8192u);
+  else if (Context.getTargetInfo().getTriple().isOSAIX())
+MaximumAlignment = std::min(MaximumAlignment, 4096u);
+
   if (AlignVal > MaximumAlignment) {
 Diag(AttrLoc, diag::err_attribute_aligned_too_great)
 << MaximumAlignment << E->getSourceRange();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

2021-08-04 Thread David Goldman via Phabricator via cfe-commits
dgoldman added a comment.

Okay I think this is what you had in mind. LMK, if it's good I'll go ahead and 
delete the other ones


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105904

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


[PATCH] D107492: [clang] Replace asm with __asm__ in cuda header

2021-08-04 Thread Evgeny Mankov via Phabricator via cfe-commits
emankov requested changes to this revision.
emankov added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:1043
 }
-#else // CUDA_VERSION >= 9020
+#else  // CUDA_VERSION >= 9020
 // CUDA no longer provides inline assembly (or bitcode) implementation of these

Unneeded formatting.



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:1057
+  __asm__("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));

Tabs are not allowed, please use whitespaces instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107492

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


[PATCH] D107506: [PowerPC][AIX] Warn when using pragma align(packed) on AIX.

2021-08-04 Thread Sean Fertile via Phabricator via cfe-commits
sfertile created this revision.
sfertile added reviewers: stevewan, Jake-Egan, cebowleratibm.
sfertile added a project: PowerPC.
Herald added subscribers: shchenz, nemanjai.
sfertile requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

With xlc and xlC pragma align(packed) will pack bitfields the same way as 
pragma align(bit_packed). xlclang, xlclang++ and clang will pack bitfields the 
same way as pragma pack(1). Issue a warning when source code using pragma 
align(packed)  to alert the user it may not be compatible with xlc/xlC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107506

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaAttr.cpp
  clang/test/Sema/aix-pragma-align-packed-warn.c
  clang/test/Sema/aix-pragma-pack-and-align.c


Index: clang/test/Sema/aix-pragma-pack-and-align.c
===
--- clang/test/Sema/aix-pragma-pack-and-align.c
+++ clang/test/Sema/aix-pragma-pack-and-align.c
@@ -170,7 +170,7 @@
 } // namespace test7
 
 namespace test8 {
-#pragma align(packed)
+#pragma align(packed) // expected-warning {{#pragma align(packed) may not be 
compatible with objects generated with AIX XL C/C++}}
 #pragma pack(2)
 #pragma pack(show) // expected-warning {{value of #pragma pack(show) == 2}}
 struct A {
Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+
+#pragma align(packed) // expected-warning {{#pragma align(packed) may not be 
compatible with objects generated with AIX XL C/C++}}
+struct A {
+  int a : 8;
+  int   : 0;
+  short s;
+};
+#pragma align(reset)
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -234,6 +234,8 @@
 // Note that '#pragma options align=packed' is not equivalent to attribute
 // packed, it has a different precedence relative to attribute aligned.
   case POAK_Packed:
+if (this->Context.getTargetInfo().getTriple().isOSAIX())
+  Diag(PragmaLoc, diag::warn_pragma_align_not_xl_compatible);
 Action = Sema::PSK_Push_Set;
 ModeVal = AlignPackInfo::Packed;
 break;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -913,6 +913,9 @@
   InGroup;
 def err_pragma_options_align_mac68k_target_unsupported : Error<
   "mac68k alignment pragma is not supported on this target">;
+def warn_pragma_align_not_xl_compatible : Warning<
+  "#pragma align(packed) may not be compatible with objects generated with AIX 
XL C/C++">,
+  InGroup;
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup;


Index: clang/test/Sema/aix-pragma-pack-and-align.c
===
--- clang/test/Sema/aix-pragma-pack-and-align.c
+++ clang/test/Sema/aix-pragma-pack-and-align.c
@@ -170,7 +170,7 @@
 } // namespace test7
 
 namespace test8 {
-#pragma align(packed)
+#pragma align(packed) // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
 #pragma pack(2)
 #pragma pack(show) // expected-warning {{value of #pragma pack(show) == 2}}
 struct A {
Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+
+#pragma align(packed) // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+struct A {
+  int a : 8;
+  int   : 0;
+  short s;
+};
+#pragma align(reset)
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -234,6 +234,8 @@
 // Note that '#pragma options align=packed' is not equivalent to attribute
 // packed, it has a different precedence relative to attribute aligned.
   case POAK_Packed:
+if (this->Context.getTargetInfo().getTriple().isOSAIX())
+  Diag(PragmaLoc, diag::warn_pragma_align_not_xl_compatible);
 Action = Sema::PSK_Push_Set;
 ModeVal = AlignPackInfo::Packed;
 break;
Index: 

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

2021-08-04 Thread David Goldman via Phabricator via cfe-commits
dgoldman updated this revision to Diff 364263.
dgoldman added a comment.

the third time is the charm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105904

Files:
  clang-tools-extra/clangd/CollectMacros.cpp
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang/include/clang/Lex/PPCallbacks.h

Index: clang/include/clang/Lex/PPCallbacks.h
===
--- clang/include/clang/Lex/PPCallbacks.h
+++ clang/include/clang/Lex/PPCallbacks.h
@@ -492,6 +492,11 @@
 Second->PragmaComment(Loc, Kind, Str);
   }
 
+  void PragmaMark(SourceLocation Loc, StringRef Trivia) override {
+First->PragmaMark(Loc, Trivia);
+Second->PragmaMark(Loc, Trivia);
+  }
+
   void PragmaDetectMismatch(SourceLocation Loc, StringRef Name,
 StringRef Value) override {
 First->PragmaDetectMismatch(Loc, Name, Value);
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -99,6 +99,8 @@
   return arg.beginOffset() == R.Begin && arg.endOffset() == R.End;
 }
 
+MATCHER_P(PragmaTrivia, P, "") { return arg.Trivia == P; }
+
 MATCHER(EqInc, "") {
   Inclusion Actual = testing::get<0>(arg);
   Inclusion Expected = testing::get<1>(arg);
@@ -886,6 +888,27 @@
   EXPECT_FALSE(mainIsGuarded(AST));
 }
 
+TEST(ParsedASTTest, DiscoversPragmaMarks) {
+  TestTU TU;
+  TU.AdditionalFiles["Header.h"] = R"(
+#pragma mark - Something API
+int something();
+#pragma mark Something else
+  )";
+  TU.Code = R"cpp(
+#include "Header.h"
+#pragma mark In Preamble
+#pragma mark - Something Impl
+int something() { return 1; }
+#pragma mark End
+  )cpp";
+  auto AST = TU.build();
+
+  EXPECT_THAT(AST.getMarks(), ElementsAre(PragmaTrivia(" In Preamble"),
+  PragmaTrivia(" - Something Impl"),
+  PragmaTrivia(" End")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -1027,6 +1027,75 @@
 AllOf(WithName("-pur"), WithKind(SymbolKind::Method));
 }
 
+TEST(DocumentSymbolsTest, PragmaMarkGroups) {
+  TestTU TU;
+  TU.ExtraArgs = {"-xobjective-c++", "-Wno-objc-root-class"};
+  Annotations Main(R"cpp(
+  $DogDef[[@interface Dog
+  @end]]
+
+  $DogImpl[[@implementation Dog
+
+  + (id)sharedDoggo { return 0; }
+
+  #pragma $Overrides[[mark - Overrides
+
+  - (id)init {
+return self;
+  }
+  - (void)bark {}]]
+
+  #pragma $Specifics[[mark - Dog Specifics
+
+  - (int)isAGoodBoy {
+return 1;
+  }]]
+  @]]end  // FIXME: Why doesn't this include the 'end'?
+
+  #pragma $End[[mark - End
+]]
+)cpp");
+  TU.Code = Main.code().str();
+  EXPECT_THAT(
+  getSymbols(TU.build()),
+  UnorderedElementsAre(
+  AllOf(WithName("Dog"), SymRange(Main.range("DogDef"))),
+  AllOf(WithName("Dog"), SymRange(Main.range("DogImpl")),
+Children(AllOf(WithName("+sharedDoggo"),
+   WithKind(SymbolKind::Method)),
+ AllOf(WithName("Overrides"),
+   SymRange(Main.range("Overrides")),
+   Children(AllOf(WithName("-init"),
+  WithKind(SymbolKind::Method)),
+AllOf(WithName("-bark"),
+  WithKind(SymbolKind::Method,
+ AllOf(WithName("Dog Specifics"),
+   SymRange(Main.range("Specifics")),
+   Children(AllOf(WithName("-isAGoodBoy"),
+  WithKind(SymbolKind::Method)),
+  AllOf(WithName("End"), SymRange(Main.range("End");
+}
+
+TEST(DocumentSymbolsTest, PragmaMarkGroupsNoNesting) {
+  TestTU TU;
+  TU.ExtraArgs = {"-xobjective-c++", "-Wno-objc-root-class"};
+  Annotations Main(R"cpp(
+  #pragma mark Helpers
+  void helpA(id obj) {}
+
+

[PATCH] D107503: [clang][Rewriter] patch to fix bug with ReplaceText erasing too many characters when text has been inserted before the replace location. See https://llvm.discourse.group/t/possible-bu

2021-08-04 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon created this revision.
ajohnson-uoregon added reviewers: jdoerfert, rsmith, klimek.
ajohnson-uoregon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

...-rewriterope/3970

[clang][Rewriter] split test for replace after insert to be more understandable 
and also a fix to make the test correct


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107503

Files:
  clang/include/clang/Rewrite/Core/Rewriter.h
  clang/lib/Rewrite/Rewriter.cpp
  clang/unittests/Rewrite/RewriterTest.cpp

Index: clang/unittests/Rewrite/RewriterTest.cpp
===
--- clang/unittests/Rewrite/RewriterTest.cpp
+++ clang/unittests/Rewrite/RewriterTest.cpp
@@ -62,6 +62,7 @@
   // Check that correct text is replaced for each range type.  Ranges remain in
   // terms of the original text but include the new text.
   StringRef Code = "int main(int argc, char *argv[]) { return argc; }";
+  //  foogc; }
   //replace char range with "foo" ^~
   //  get ^ = "foogc;"
   //   replace token range with "bar" ^~++
@@ -77,4 +78,54 @@
   EXPECT_EQ(T.Rewrite.getRewrittenText(T.makeCharRange(42, 47)), "0;");
 }
 
+TEST(Rewriter, ReplaceAfterInsertNoOpts) {
+  // Check that remove/insert after insert behaves correctly based on the
+  // RewriteOptions passed. This should erase extra text.
+
+  StringRef code = "int main(int argc, char *argv[]) { return argc; } double "
+   "f(int text_to_erase) {}";
+  //insert "int answer = 42;" ^
+  //  replace with 42 ^
+  // extra text to erase ^
+  std::unique_ptr AST = tooling::buildASTFromCode(code);
+  ASTContext  = AST->getASTContext();
+  Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());
+  SourceLocation FileStart = AST->getStartOfMainFileID();
+  SourceLocation insert_point = FileStart.getLocWithOffset(34);
+  Rewrite.InsertText(insert_point, "int answer = 42;");
+  SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(34),
+  FileStart.getLocWithOffset(45));
+  Rewrite.ReplaceText(replace_range, "42;");
+  EXPECT_EQ(
+  Rewrite.getRewrittenText(SourceRange(FileStart.getLocWithOffset(33),
+   FileStart.getLocWithOffset(65))),
+  "{int answer = 42;42; text_to_erase");
+}
+
+TEST(Rewriter, ReplaceAfterInsertOpts) {
+  // Check that remove/insert after insert behaves correctly based on the
+  // RewriteOptions passed. This should NOT erase extra text.
+
+  StringRef code = "int main(int argc, char *argv[]) { return argc; } double "
+   "f(int text_to_erase) {}";
+  //insert "int answer = 42;" ^
+  //  replace with 42 ^
+  // extra text to erase ^
+  std::unique_ptr AST = tooling::buildASTFromCode(code);
+  ASTContext  = AST->getASTContext();
+  Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());
+  SourceLocation FileStart = AST->getStartOfMainFileID();
+  SourceLocation insert_point = FileStart.getLocWithOffset(34);
+  Rewrite.InsertText(insert_point, "int answer = 42;");
+  SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(34),
+  FileStart.getLocWithOffset(46));
+  Rewriter::RewriteOptions opts;
+  opts.IncludeInsertsAtBeginOfRange = false;
+  Rewrite.ReplaceText(replace_range, "42;", opts);
+  EXPECT_EQ(
+  Rewrite.getRewrittenText(SourceRange(FileStart.getLocWithOffset(33),
+   FileStart.getLocWithOffset(58))),
+  "{int answer = 42;42; } double f(");
+}
+
 } // anonymous namespace
Index: clang/lib/Rewrite/Rewriter.cpp
===
--- clang/lib/Rewrite/Rewriter.cpp
+++ clang/lib/Rewrite/Rewriter.cpp
@@ -327,13 +327,13 @@
   return false;
 }
 
-bool Rewriter::ReplaceText(SourceRange range, SourceRange replacementRange) {
+bool Rewriter::ReplaceText(SourceRange range, SourceRange replacementRange, RewriteOptions opts) {
   if (!isRewritable(range.getBegin())) return true;
   if (!isRewritable(range.getEnd())) return true;
   if (replacementRange.isInvalid()) return true;
   SourceLocation start = range.getBegin();
-  unsigned origLength = getRangeSize(range);
-  unsigned newLength = getRangeSize(replacementRange);
+  unsigned origLength = getRangeSize(range, opts);
+  unsigned newLength = getRangeSize(replacementRange, opts);
   FileID FID;
   unsigned newOffs = getLocationOffsetAndFileID(replacementRange.getBegin(),
 FID);
Index: 

[PATCH] D107318: [OpenCL] allow generic address and non-generic defs for CL3.0

2021-08-04 Thread Dave Airlie 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 rG14cb67862a72: [OpenCL] allow generic address and non-generic 
defs for CL3.0 (authored by airlied).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107318

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

Index: clang/lib/Headers/opencl-c.h
===
--- clang/lib/Headers/opencl-c.h
+++ clang/lib/Headers/opencl-c.h
@@ -13300,7 +13300,8 @@
 void __ovld atomic_init(volatile atomic_double *object, double value);
 #endif //cl_khr_fp64
 #endif
-#elif __OPENCL_C_VERSION__ >= CL_VERSION_3_0
+#endif //defined(__opencl_c_generic_address_space)
+#if __OPENCL_C_VERSION__ >= CL_VERSION_3_0
 void __ovld atomic_init(volatile __global atomic_int *object, int value);
 void __ovld atomic_init(volatile __local atomic_int *object, int value);
 void __ovld atomic_init(volatile __global atomic_uint *object, uint value);
@@ -13360,7 +13361,8 @@
 uintptr_t __ovld atomic_fetch_add(volatile atomic_uintptr_t *object, ptrdiff_t operand);
 uintptr_t __ovld atomic_fetch_sub(volatile atomic_uintptr_t *object, ptrdiff_t operand);
 #endif //defined(cl_khr_int64_base_atomics) && defined(cl_khr_int64_extended_atomics)
-#elif __OPENCL_C_VERSION__ >= CL_VERSION_3_0
+#endif //defined(__opencl_c_generic_address_space)
+#if __OPENCL_C_VERSION__ >= CL_VERSION_3_0
 int __ovld atomic_fetch_add(volatile __global atomic_int *object, int operand);
 int __ovld atomic_fetch_add(volatile __local atomic_int *object, int operand);
 uint __ovld atomic_fetch_add(volatile __global atomic_uint *object, uint operand);
@@ -13478,7 +13480,8 @@
 uintptr_t __ovld atomic_fetch_add_explicit(volatile atomic_uintptr_t *object, ptrdiff_t operand, memory_order order);
 uintptr_t __ovld atomic_fetch_sub_explicit(volatile atomic_uintptr_t *object, ptrdiff_t operand, memory_order order);
 #endif //defined(cl_khr_int64_base_atomics) && defined(cl_khr_int64_extended_atomics)
-#elif __OPENCL_C_VERSION__ >= CL_VERSION_3_0
+#endif //defined(__opencl_c_generic_address_space)
+#if __OPENCL_C_VERSION__ >= CL_VERSION_3_0
 int __ovld atomic_fetch_add_explicit(volatile __global atomic_int *object, int operand, memory_order order);
 int __ovld atomic_fetch_add_explicit(volatile __local atomic_int *object, int operand, memory_order order);
 uint __ovld atomic_fetch_add_explicit(volatile __global atomic_uint *object, uint operand, memory_order order);
@@ -13595,7 +13598,8 @@
 uintptr_t __ovld atomic_fetch_add_explicit(volatile atomic_uintptr_t *object, ptrdiff_t operand, memory_order order, memory_scope scope);
 uintptr_t __ovld atomic_fetch_sub_explicit(volatile atomic_uintptr_t *object, ptrdiff_t operand, memory_order order, memory_scope scope);
 #endif
-#elif __OPENCL_C_VERSION__ >= CL_VERSION_3_0
+#endif //defined(__opencl_c_generic_address_space)
+#if __OPENCL_C_VERSION__ >= CL_VERSION_3_0
 int __ovld atomic_fetch_add_explicit(volatile __global atomic_int *object, int operand, memory_order order, memory_scope scope);
 int __ovld atomic_fetch_add_explicit(volatile __local atomic_int *object, int operand, memory_order order, memory_scope scope);
 uint __ovld atomic_fetch_add_explicit(volatile __global atomic_uint *object, uint operand, memory_order order, memory_scope scope);
@@ -13693,7 +13697,8 @@
 void __ovld atomic_store(volatile atomic_long *object, long desired);
 void __ovld atomic_store(volatile atomic_ulong *object, ulong desired);
 #endif
-#elif __OPENCL_C_VERSION__ >= CL_VERSION_3_0
+#endif //defined(__opencl_c_generic_address_space)
+#if __OPENCL_C_VERSION__ >= CL_VERSION_3_0
 void __ovld atomic_store(volatile __global atomic_int *object, int desired);
 void __ovld atomic_store(volatile __local atomic_int *object, int desired);
 void __ovld atomic_store(volatile __global atomic_uint *object, uint desired);
@@ -13725,7 +13730,8 @@
 void __ovld atomic_store_explicit(volatile atomic_long *object, long desired, memory_order order);
 void __ovld atomic_store_explicit(volatile atomic_ulong *object, ulong desired, memory_order order);
 #endif
-#elif __OPENCL_C_VERSION__ >= CL_VERSION_3_0
+#endif //defined(__opencl_c_generic_address_space)
+#if __OPENCL_C_VERSION__ >= CL_VERSION_3_0
 void __ovld atomic_store_explicit(volatile __global atomic_int *object, int desired, memory_order order);
 void __ovld atomic_store_explicit(volatile __local atomic_int *object, int desired, memory_order order);
 void __ovld atomic_store_explicit(volatile __global atomic_uint *object, uint desired, memory_order order);
@@ -13756,7 +13762,8 @@
 void __ovld atomic_store_explicit(volatile atomic_long *object, long desired, memory_order order, memory_scope scope);
 void __ovld atomic_store_explicit(volatile atomic_ulong *object, ulong desired, memory_order order, memory_scope scope);
 #endif
-#elif __OPENCL_C_VERSION__ >= 

[clang] 14cb678 - [OpenCL] allow generic address and non-generic defs for CL3.0

2021-08-04 Thread Dave Airlie via cfe-commits

Author: Dave Airlie
Date: 2021-08-05T07:32:45+10:00
New Revision: 14cb67862a723027c6787baa263f5bf6e03ab01d

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

LOG: [OpenCL] allow generic address and non-generic defs for CL3.0

 This allows both sets of definitions to exist on CL 3.0

Reviewed By: Anastasia

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Headers/opencl-c.h b/clang/lib/Headers/opencl-c.h
index fa39bf2261f5..e348a092f092 100644
--- a/clang/lib/Headers/opencl-c.h
+++ b/clang/lib/Headers/opencl-c.h
@@ -13300,7 +13300,8 @@ void __ovld atomic_init(volatile atomic_ulong *object, 
ulong value);
 void __ovld atomic_init(volatile atomic_double *object, double value);
 #endif //cl_khr_fp64
 #endif
-#elif __OPENCL_C_VERSION__ >= CL_VERSION_3_0
+#endif //defined(__opencl_c_generic_address_space)
+#if __OPENCL_C_VERSION__ >= CL_VERSION_3_0
 void __ovld atomic_init(volatile __global atomic_int *object, int value);
 void __ovld atomic_init(volatile __local atomic_int *object, int value);
 void __ovld atomic_init(volatile __global atomic_uint *object, uint value);
@@ -13360,7 +13361,8 @@ ulong __ovld atomic_fetch_max(volatile atomic_ulong 
*object, ulong operand);
 uintptr_t __ovld atomic_fetch_add(volatile atomic_uintptr_t *object, ptr
diff _t operand);
 uintptr_t __ovld atomic_fetch_sub(volatile atomic_uintptr_t *object, ptr
diff _t operand);
 #endif //defined(cl_khr_int64_base_atomics) && 
defined(cl_khr_int64_extended_atomics)
-#elif __OPENCL_C_VERSION__ >= CL_VERSION_3_0
+#endif //defined(__opencl_c_generic_address_space)
+#if __OPENCL_C_VERSION__ >= CL_VERSION_3_0
 int __ovld atomic_fetch_add(volatile __global atomic_int *object, int operand);
 int __ovld atomic_fetch_add(volatile __local atomic_int *object, int operand);
 uint __ovld atomic_fetch_add(volatile __global atomic_uint *object, uint 
operand);
@@ -13478,7 +13480,8 @@ ulong __ovld atomic_fetch_max_explicit(volatile 
atomic_ulong *object, ulong oper
 uintptr_t __ovld atomic_fetch_add_explicit(volatile atomic_uintptr_t *object, 
ptr
diff _t operand, memory_order order);
 uintptr_t __ovld atomic_fetch_sub_explicit(volatile atomic_uintptr_t *object, 
ptr
diff _t operand, memory_order order);
 #endif //defined(cl_khr_int64_base_atomics) && 
defined(cl_khr_int64_extended_atomics)
-#elif __OPENCL_C_VERSION__ >= CL_VERSION_3_0
+#endif //defined(__opencl_c_generic_address_space)
+#if __OPENCL_C_VERSION__ >= CL_VERSION_3_0
 int __ovld atomic_fetch_add_explicit(volatile __global atomic_int *object, int 
operand, memory_order order);
 int __ovld atomic_fetch_add_explicit(volatile __local atomic_int *object, int 
operand, memory_order order);
 uint __ovld atomic_fetch_add_explicit(volatile __global atomic_uint *object, 
uint operand, memory_order order);
@@ -13595,7 +13598,8 @@ ulong __ovld atomic_fetch_max_explicit(volatile 
atomic_ulong *object, ulong oper
 uintptr_t __ovld atomic_fetch_add_explicit(volatile atomic_uintptr_t *object, 
ptr
diff _t operand, memory_order order, memory_scope scope);
 uintptr_t __ovld atomic_fetch_sub_explicit(volatile atomic_uintptr_t *object, 
ptr
diff _t operand, memory_order order, memory_scope scope);
 #endif
-#elif __OPENCL_C_VERSION__ >= CL_VERSION_3_0
+#endif //defined(__opencl_c_generic_address_space)
+#if __OPENCL_C_VERSION__ >= CL_VERSION_3_0
 int __ovld atomic_fetch_add_explicit(volatile __global atomic_int *object, int 
operand, memory_order order, memory_scope scope);
 int __ovld atomic_fetch_add_explicit(volatile __local atomic_int *object, int 
operand, memory_order order, memory_scope scope);
 uint __ovld atomic_fetch_add_explicit(volatile __global atomic_uint *object, 
uint operand, memory_order order, memory_scope scope);
@@ -13693,7 +13697,8 @@ void __ovld atomic_store(volatile atomic_double 
*object, double desired);
 void __ovld atomic_store(volatile atomic_long *object, long desired);
 void __ovld atomic_store(volatile atomic_ulong *object, ulong desired);
 #endif
-#elif __OPENCL_C_VERSION__ >= CL_VERSION_3_0
+#endif //defined(__opencl_c_generic_address_space)
+#if __OPENCL_C_VERSION__ >= CL_VERSION_3_0
 void __ovld atomic_store(volatile __global atomic_int *object, int desired);
 void __ovld atomic_store(volatile __local atomic_int *object, int desired);
 void __ovld atomic_store(volatile __global atomic_uint *object, uint desired);
@@ -13725,7 +13730,8 @@ void __ovld atomic_store_explicit(volatile 
atomic_double *object, double desired
 void __ovld atomic_store_explicit(volatile atomic_long *object, long desired, 
memory_order order);
 void __ovld atomic_store_explicit(volatile atomic_ulong *object, ulong 
desired, memory_order order);
 #endif
-#elif __OPENCL_C_VERSION__ >= 

[PATCH] D106960: [OffloadArch] Library to query properties of current offload archicture

2021-08-04 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

This only works on Linux. So either to make it work on both Linux and Windows, 
or restrict it to Linux in CMakeLists.txt, otherwise it breaks LLVM build on 
Windows.




Comment at: llvm/lib/OffloadArch/OffloadArch.cpp:17
+#include "llvm/Support/WithColor.h"
+#include 
+#include 

better to use LLVM or standard C++ functions for directory operations since 
dirent.h is not available in MSVC. Even though this utility only works on Linux 
for now, it is better to be platform-neutral to be ported to Windows.



Comment at: llvm/lib/OffloadArch/amdgpu/vendor_specific_capabilities.cpp:25
+//
+#include "hsa-subset.h"
+#include 

It would be much simpler to use HIP API to get device name and capabilities 
e.g. gfx906:xnack+:sramecc-

https://github.com/ROCm-Developer-Tools/HIP/blob/rocm-4.2.x/samples/1_Utils/hipInfo/hipInfo.cpp

It will work on both Linux and Windows. On Linux the availability of HIP 
runtime is the same as HSA runtime. On Windows HIP runtime is shipped with 
display driver, whereas HSA runtime is not available.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106960

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


[PATCH] D106713: Thread safety analysis: Warn when demoting locks on back edges

2021-08-04 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment.

Ping @delesley.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106713

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


[PATCH] D107430: [OMPIRBuilder] Add ordered without depend and simd clause to OMPBuilder

2021-08-04 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:708
   ///
-  /// \returns The insertion position *after* the master.
+  /// \returns The insertion position *after* the masked.
   InsertPointTy createMasked(const LocationDescription ,

Could you commit these typo fixes separately? That is, just push it to main, no 
Phabricator review required.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:721
   ///
-  /// \returns The insertion position *after* the master.
+  /// \returns The insertion position *after* the critical.
   InsertPointTy createCritical(const LocationDescription ,

Could you commit these typo fixes separately? That is, just push it to main, no 
Phabricator review required.



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2125
+
+  AllocaInst *PrivAI = Builder.CreateAlloca(F->arg_begin()->getType());
+

Could you give this alloca register a name?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107430

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


[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

2021-08-04 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment.

Files bug to edit the release not as suggested by @xbolva00 here: 
https://bugs.llvm.org/show_bug.cgi?id=51347


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74436

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


[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

2021-08-04 Thread Haowei Wu via Phabricator via cfe-commits
haowei added a comment.

In D74436#2926385 , @tstellar wrote:

> Can someone file a bug for this and put release-13.0.0 in the blocks field so 
> we can track it?

Filed https://bugs.llvm.org/show_bug.cgi?id=51346 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74436

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


[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

2021-08-04 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko accepted this revision.
vsavchenko added a comment.

In D107366#2926662 , @ASDenysPetrov 
wrote:

> In D107366#2926343 , @NoQ wrote:
>
>> Works in Firefox on macOS as well!
>
> Great!
> @vsavchenko , any suggestions?

LGTM!  Thanks fo taking care of this!


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

https://reviews.llvm.org/D107366

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


[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

2021-08-04 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment.

In D107366#2926343 , @NoQ wrote:

> Works in Firefox on macOS as well!

Great!
@vsavchenko , any suggestions?


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

https://reviews.llvm.org/D107366

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


[PATCH] D107492: [clang] Replace asm with __asm__ in cuda header

2021-08-04 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM in general.




Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:37
 #if defined(__cplusplus)
-__DEVICE__ void __brkpt() { asm volatile("brkpt;"); }
+__DEVICE__ void __brkpt() { __asm__ volatile("brkpt;"); }
 __DEVICE__ void __brkpt(int __a) { __brkpt(); }

Should we also change `volatile` ->  `__volatile__` here in other places in the 
file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107492

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


[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

2021-08-04 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added inline comments.



Comment at: clang/lib/AST/ASTDumper.cpp:199
 P.setDeserialize(Deserialize);
+P.doGetNodeDelegate().setDeserialize(Deserialize);
 P.Visit(this);

It is possible it will complicate the implementation too much but what if 
`P.setDeserialize(Deserialize);` is responsible for propagating `Deserialize` 
to its NodeDelegate?


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

https://reviews.llvm.org/D80878

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


[PATCH] D107497: [PowerPC][AIX] Limit attribute aligned to 4096.

2021-08-04 Thread Sean Fertile via Phabricator via cfe-commits
sfertile created this revision.
sfertile added reviewers: Jake-Egan, stevewan.
sfertile added a project: PowerPC.
Herald added subscribers: shchenz, nemanjai.
Herald added a reviewer: aaron.ballman.
sfertile requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Limit the maximum alignment for attribute aligned to 4096 to match the limit of 
the .align pseudo op in the system assembler.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107497

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/aix-attr-aligned-limit.c


Index: clang/test/Sema/aix-attr-aligned-limit.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-limit.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -fsyntax-only -verify %s
+//
+int a __attribute__((aligned(8192))); // expected-error {{requested alignment 
must be 4096 bytes or smaller}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4054,6 +4054,9 @@
   unsigned MaximumAlignment = Sema::MaximumAlignment;
   if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF())
 MaximumAlignment = std::min(MaximumAlignment, 8192u);
+  else if(Context.getTargetInfo().getTriple().isOSAIX())
+MaximumAlignment = std::min(MaximumAlignment, 4096u);
+
   if (AlignVal > MaximumAlignment) {
 Diag(AttrLoc, diag::err_attribute_aligned_too_great)
 << MaximumAlignment << E->getSourceRange();


Index: clang/test/Sema/aix-attr-aligned-limit.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-limit.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -fsyntax-only -verify %s
+//
+int a __attribute__((aligned(8192))); // expected-error {{requested alignment must be 4096 bytes or smaller}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4054,6 +4054,9 @@
   unsigned MaximumAlignment = Sema::MaximumAlignment;
   if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF())
 MaximumAlignment = std::min(MaximumAlignment, 8192u);
+  else if(Context.getTargetInfo().getTriple().isOSAIX())
+MaximumAlignment = std::min(MaximumAlignment, 4096u);
+
   if (AlignVal > MaximumAlignment) {
 Diag(AttrLoc, diag::err_attribute_aligned_too_great)
 << MaximumAlignment << E->getSourceRange();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D106030: [Clang] add support for error+warning fn attrs

2021-08-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:6057
+assertions that depend on optimizations, while providing diagnostics
+pointing to precise locations of the call site in the source.
+  }];

nickdesaulniers wrote:
> @aaron.ballman anything I should add to the documentation blurb?
I think the prose is generally fine, but it might be useful to show some 
trivial example usages to set expectations. e.g.,
```
__attribute__((warning("oh no"))) void unused_func() {} // Not diagnosed 
because it's never called
__attribute__((warning("oh no"))) void used_func() {}
__attribute__((error("oh no"))) int unevaluated_func() { return 0; }
__attribute__((error("oh no"))) constexpr int constexpr_func() { return 12; }

void use() {
  used_func(); // Warning
  sizeof(unevaluated_func()); // No error, function never called
  int array[constexpr_func()]; // No error, function never called
}
```
Feel free to use better/different/more examples, btw. I'm not tied to mine.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1205
 def BackendOptimizationFailure : DiagGroup<"pass-failed">;
+def BackendUserDiagnostic : DiagGroup<"user-diagnostic">;
 

nickdesaulniers wrote:
> aaron.ballman wrote:
> > nickdesaulniers wrote:
> > > aaron.ballman wrote:
> > > > nickdesaulniers wrote:
> > > > > aaron.ballman wrote:
> > > > > > GCC doesn't seem to support this spelling -- do they have a 
> > > > > > different one we should reuse?
> > > > > I think these diagnostics don't have corresponding flags in GCC, so 
> > > > > they cannot be disabled.
> > > > > 
> > > > > Without adding this, clang/test/Misc/warning-flags.c would fail, 
> > > > > because I was adding a new unnamed warning 
> > > > > `warn_fe_backend_user_diagnostic`.  Perhaps I should not be adding 
> > > > > this line here, and doing something else?
> > > > > 
> > > > > That test says we shouldn't be adding new warnings not controlled by 
> > > > > flags, but that's very much my intention.
> > > > > 
> > > > > Though now `-Wno-user-diagnostic` doesn't produce a 
> > > > > `-Wunknown-warning-option` diagnostic...
> > > > Ah! I think the warning attribute should be controllable via a 
> > > > diagnostic flag (we should always be able to disable warnings with some 
> > > > sort of flag) and I don't think the error attribute should be 
> > > > controllable (an error is an error and should stop translation, same as 
> > > > with `#error`).
> > > > 
> > > > Normally, I'd say "let's use the same flag that controls `#warning`" 
> > > > but...
> > > > ```
> > > > def PoundWarning : DiagGroup<"#warnings">;
> > > > ```
> > > > That's... not exactly an obvious flag for the warning attribute. So I 
> > > > would probably name it `warning-attributes` similar to how we have 
> > > > `deprecated-attributes` already.
> > > Done, now `-Wno-warning-attributes` doesn't produce 
> > > `-Wunknown-warning-option`, but also doesn't disable the diagnostic.  Was 
> > > there something else I needed to add?
> > huh, that sounds suspicious -- you don't need to do anything special for 
> > `-Wno-foo` handling, that should be automatically supported via tablegen. 
> > I'm not certain why you're not seeing `-Wno-warning-attributes` silencing 
> > the warnings.
> Ah! Because I was testing `__attribute__((error("")))` not 
> `__attribute__((warning("")))`. `-Wno-warning-attributes` only affects the 
> latter, not the former.  I should add a test for `-Wno-warning-attributes`! 
> Is this something I also need to add documentation for?
Given that this behavior surprised you, I certainly wouldn't oppose mentioning 
it in the documentation, but I also don't think it's strictly required. That 
said, I do agree about adding some additional test coverage for when the 
warning is disabled.

Just to double-check: do you think this functionality needs an "escape hatch" 
for the error case? e.g., should the `error` attribute generate a warning 
diagnostic that defaults to being an error, so we have 
`-Wno-warning-attributes` to turn off `warning` attribute diagnostics and 
`-Wno-error-attributes` to turn off `error` attribute diagnostics?



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:783
+  else
+Diags.Report(DiagID) << CalleeName << D.getMsgStr();
+}

nickdesaulniers wrote:
> aaron.ballman wrote:
> > nickdesaulniers wrote:
> > > aaron.ballman wrote:
> > > > nickdesaulniers wrote:
> > > > > nickdesaulniers wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > nickdesaulniers wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > I think the usability in this situation is a concern -- only 
> > > > > > > > > having a function name but no source location information is 
> > > > > > > > > pretty tricky. Do you have a sense for how often we would hit 
> > > > > > > > > this case?
> > > > > > > > I don't think we ever encounter it when building the kernel 
> 

[PATCH] D106701: [clang] Implement -falign-loops=N (N is a power of 2) for non-LTO

2021-08-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D106701#2924638 , @luismarques 
wrote:

> Still LGTM.
>
> BTW, I liked that in the old version the help string included "In GCC =0 is 
> the same as =1".

I find that in GCC =0 is not necessarily =1, but not particular clear about its 
exact behavior;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106701

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


[PATCH] D106701: [clang] Implement -falign-loops=N (N is a power of 2) for non-LTO

2021-08-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 364224.
MaskRay retitled this revision from "[clang] Add -falign-loops=N where N is a 
power of 2" to "[clang] Implement -falign-loops=N (N is a power of 2) for 
non-LTO".
MaskRay edited the summary of this revision.
MaskRay added a comment.
Herald added a subscriber: inglorion.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106701

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/align-loops.c
  clang/test/Driver/falign-loops.c

Index: clang/test/Driver/falign-loops.c
===
--- /dev/null
+++ clang/test/Driver/falign-loops.c
@@ -0,0 +1,17 @@
+/// Treat -falign-loops=0 as not specifying the option.
+// RUN: %clang -### -falign-loops=0 %s 2>&1 | FileCheck %s --check-prefix=CHECK_NO
+// RUN: %clang -### -falign-loops=1 %s 2>&1 | FileCheck %s --check-prefix=CHECK_1
+// RUN: %clang -### -falign-loops=4 %s 2>&1 | FileCheck %s --check-prefix=CHECK_4
+/// Only powers of 2 are supported for now.
+// RUN: %clang -### -falign-loops=5 %s 2>&1 | FileCheck %s --check-prefix=CHECK_5
+// RUN: %clang -### -falign-loops=65536 %s 2>&1 | FileCheck %s --check-prefix=CHECK_65536
+// RUN: %clang -### -falign-loops=65537 %s 2>&1 | FileCheck %s --check-prefix=CHECK_65537
+// RUN: %clang -### -falign-loops=a %s 2>&1 | FileCheck %s --check-prefix=CHECK_ERR_A
+
+// CHECK_NO-NOT: "-falign-loops=
+// CHECK_1: "-falign-loops=1"
+// CHECK_4: "-falign-loops=4"
+// CHECK_5: error: alignment is not a power of 2 in '-falign-loops=5'
+// CHECK_65536: "-falign-loops=65536"
+// CHECK_65537: error: invalid integral value '65537' in '-falign-loops=65537'
+// CHECK_ERR_A: error: invalid integral value 'a' in '-falign-loops=a'
Index: clang/test/CodeGen/align-loops.c
===
--- /dev/null
+++ clang/test/CodeGen/align-loops.c
@@ -0,0 +1,15 @@
+// REQUIRES: x86-registered-target
+/// Check asm because we use llvm::TargetOptions.
+
+// RUN: %clang_cc1 -triple=x86_64 -S %s -falign-loops=8 -O -o - | FileCheck %s --check-prefixes=CHECK,CHECK_8
+// RUN: %clang_cc1 -triple=x86_64 -S %s -falign-loops=32 -O -o - | FileCheck %s --check-prefixes=CHECK,CHECK_32
+
+// CHECK-LABEL: foo:
+// CHECK_8:   .p2align 3, 0x90
+// CHECK_32:  .p2align 5, 0x90
+
+void bar();
+void foo() {
+  for (int i = 0; i < 64; ++i)
+bar();
+}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4739,6 +4739,22 @@
 CmdArgs.push_back(Args.MakeArgString(std::to_string(FunctionAlignment)));
   }
 
+  // We support -falign-loops=N where N is a power of 2. GCC supports more
+  // forms.
+  if (const Arg *A = Args.getLastArg(options::OPT_falign_loops_EQ)) {
+unsigned Value = 0;
+if (StringRef(A->getValue()).getAsInteger(10, Value) || Value > 65536)
+  TC.getDriver().Diag(diag::err_drv_invalid_int_value)
+  << A->getAsString(Args) << A->getValue();
+else if (Value & Value - 1)
+  TC.getDriver().Diag(diag::err_drv_alignment_not_power_of_two)
+  << A->getAsString(Args) << A->getValue();
+// Treat =0 as unspecified (use the target preference).
+if (Value)
+  CmdArgs.push_back(Args.MakeArgString("-falign-loops=" +
+   Twine(std::min(Value, 65536u;
+  }
+
   llvm::Reloc::Model RelocationModel;
   unsigned PICLevel;
   bool IsPIE;
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -580,6 +580,7 @@
   Options.ValueTrackingVariableLocations =
   CodeGenOpts.ValueTrackingVariableLocations;
   Options.XRayOmitFunctionIndex = CodeGenOpts.XRayOmitFunctionIndex;
+  Options.LoopAlignment = CodeGenOpts.LoopAlignment;
 
   Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;
   Options.MCOptions.MCRelaxAll = CodeGenOpts.RelaxAll;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1066,6 +1066,9 @@
   PosFlag>;
 def falign_functions : Flag<["-"], "falign-functions">, Group;
 def falign_functions_EQ : Joined<["-"], "falign-functions=">, Group;
+def falign_loops_EQ : Joined<["-"], "falign-loops=">, Group, Flags<[CC1Option]>, MetaVarName<"">,
+  HelpText<"N must be a power of two. Align loops to the boundary">,
+  MarshallingInfoInt>;
 def fno_align_functions: Flag<["-"], 

[PATCH] D107138: [PowerPC] Implement cmplxl builtins

2021-08-04 Thread Albion Fung via Phabricator via cfe-commits
Conanap marked an inline comment as done.
Conanap added inline comments.



Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-complex.c:1
+// RUN: %clang_cc1 -O2 -triple powerpc64-unknown-unknown \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr7 | FileCheck %s

NeHuang wrote:
> NeHuang wrote:
> > `// REQUIRES: powerpc-registered-target`
> Question: why do we need `-O2` for this builtin?
> 
it's not required, but removes a lot of the extra load and stores that make the 
test cases longer unnecessarily. I can change it to O1 if preferred.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107138

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


[PATCH] D107138: [PowerPC] Implement cmplxl builtins

2021-08-04 Thread Albion Fung via Phabricator via cfe-commits
Conanap updated this revision to Diff 364223.
Conanap marked 6 inline comments as done.
Conanap added a comment.

Removed backend tests, removed some uneeded definitions,
updated frontend test with regex for variable names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107138

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/CodeGen/builtins-ppc-xlcompat-complex.c


Index: clang/test/CodeGen/builtins-ppc-xlcompat-complex.c
===
--- /dev/null
+++ clang/test/CodeGen/builtins-ppc-xlcompat-complex.c
@@ -0,0 +1,30 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -O2 -triple powerpc64-unknown-unknown \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr7 | FileCheck %s
+// RUN: %clang_cc1 -O2 -triple powerpc64le-unknown-unknown \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr8 | FileCheck %s
+// RUN: %clang_cc1 -O2 -triple powerpc-unknown-aix \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr7 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-AIX
+// RUN: %clang_cc1 -O2 -triple powerpc64-unknown-aix \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr7 | FileCheck %s \
+// RUN:   --check-prefix=CHECK-AIX
+
+extern long double lda, ldb;
+
+long double _Complex test_xl_cmplxl() {
+  // CHECK-LABEL: test_xl_cmplxl
+  // CHECK: %0 = load ppc_fp128, ppc_fp128* @lda
+  // CHECK-NEXT: %1 = load ppc_fp128, ppc_fp128* @ldb
+  // CHECK-NEXT: [[VAR1:%.*]] = insertvalue { ppc_fp128, ppc_fp128 } undef, 
ppc_fp128 %0, 0
+  // CHECK-NEXT: [[VAR2:%.*]] = insertvalue { ppc_fp128, ppc_fp128 } [[VAR1]], 
ppc_fp128 %1, 1
+  // CHECK-NEXT: ret { ppc_fp128, ppc_fp128 } [[VAR2]]
+
+  // CHECK-AIX-LABEL: test_xl_cmplxl
+  // CHECK-AIX: %0 = load double, double* @lda
+  // CHECK-AIX-NEXT: %1 = load double, double* @ldb
+  // CHECK-AIX-NEXT: [[VAR3:%.*]] = insertvalue { double, double } undef, 
double %0, 0
+  // CHECK-AIX-NEXT: [[VAR4:%.*]] = insertvalue { double, double } [[VAR3]], 
double %1, 1
+  // CHECK-AIX-NEXT: ret { double, double } [[VAR4]]
+  return __cmplxl(lda, ldb);
+}
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -235,6 +235,7 @@
   Builder.defineMacro("__frsqrtes", "__builtin_ppc_frsqrtes");
   Builder.defineMacro("__fsqrt", "__builtin_ppc_fsqrt");
   Builder.defineMacro("__fsqrts", "__builtin_ppc_fsqrts");
+  Builder.defineMacro("__cmplxl", "__builtin_complex");
 }
 
 /// PPCTargetInfo::getTargetDefines - Return a set of the PowerPC-specific


Index: clang/test/CodeGen/builtins-ppc-xlcompat-complex.c
===
--- /dev/null
+++ clang/test/CodeGen/builtins-ppc-xlcompat-complex.c
@@ -0,0 +1,30 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -O2 -triple powerpc64-unknown-unknown \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr7 | FileCheck %s
+// RUN: %clang_cc1 -O2 -triple powerpc64le-unknown-unknown \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr8 | FileCheck %s
+// RUN: %clang_cc1 -O2 -triple powerpc-unknown-aix \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr7 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-AIX
+// RUN: %clang_cc1 -O2 -triple powerpc64-unknown-aix \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr7 | FileCheck %s \
+// RUN:   --check-prefix=CHECK-AIX
+
+extern long double lda, ldb;
+
+long double _Complex test_xl_cmplxl() {
+  // CHECK-LABEL: test_xl_cmplxl
+  // CHECK: %0 = load ppc_fp128, ppc_fp128* @lda
+  // CHECK-NEXT: %1 = load ppc_fp128, ppc_fp128* @ldb
+  // CHECK-NEXT: [[VAR1:%.*]] = insertvalue { ppc_fp128, ppc_fp128 } undef, ppc_fp128 %0, 0
+  // CHECK-NEXT: [[VAR2:%.*]] = insertvalue { ppc_fp128, ppc_fp128 } [[VAR1]], ppc_fp128 %1, 1
+  // CHECK-NEXT: ret { ppc_fp128, ppc_fp128 } [[VAR2]]
+
+  // CHECK-AIX-LABEL: test_xl_cmplxl
+  // CHECK-AIX: %0 = load double, double* @lda
+  // CHECK-AIX-NEXT: %1 = load double, double* @ldb
+  // CHECK-AIX-NEXT: [[VAR3:%.*]] = insertvalue { double, double } undef, double %0, 0
+  // CHECK-AIX-NEXT: [[VAR4:%.*]] = insertvalue { double, double } [[VAR3]], double %1, 1
+  // CHECK-AIX-NEXT: ret { double, double } [[VAR4]]
+  return __cmplxl(lda, ldb);
+}
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -235,6 +235,7 @@
   Builder.defineMacro("__frsqrtes", "__builtin_ppc_frsqrtes");
   Builder.defineMacro("__fsqrt", "__builtin_ppc_fsqrt");
   Builder.defineMacro("__fsqrts", "__builtin_ppc_fsqrts");
+  Builder.defineMacro("__cmplxl", "__builtin_complex");
 }
 
 /// PPCTargetInfo::getTargetDefines - Return a set of the PowerPC-specific
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D105881: [flang][driver] Switch to `BoolFOption` for boolean options

2021-08-04 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski updated this revision to Diff 364220.
awarzynski added a comment.

Remove `EmptyKPM`, introduce `OptOutFC1FFlag` and `OptInFC1FFlag`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105881

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

Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -40,7 +40,12 @@
 ! HELP-NEXT:Specify where to find the compiled intrinsic modules
 ! HELP-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-NEXT: -flogical-abbreviations Enable logical abbreviations
-! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
+! HELP-NEXT: -fno-backslash
+! HELP-NEXT: -fno-color-diagnostics  Disable colors in diagnostics
+! HELP-NEXT: -fno-implicit-none
+! HELP-NEXT: -fno-logical-abbreviations
+! HELP-NEXT: {{[[:space:]]$}}
+! HELP-NEXT: -fno-xor-operator
 ! HELP-NEXT: -fopenacc  Enable OpenACC
 ! HELP-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
 ! HELP-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
@@ -68,6 +73,8 @@
 ! HELP-FC1-NEXT: -E Only run the preprocessor
 ! HELP-FC1-NEXT: -falternative-parameter-statement
 ! HELP-FC1-NEXT: Enable the old style PARAMETER statement
+! HELP-FC1-NEXT: -fanalyzed-objects-for-unparse
+! HELP-FC1-NEXT: {{[[:space:]]$}}
 ! HELP-FC1-NEXT: -fbackslashSpecify that backslash in string introduces an escape character
 ! HELP-FC1-NEXT: -fdebug-dump-all   Dump symbols and the parse tree after the semantic checks
 ! HELP-FC1-NEXT: -fdebug-dump-parse-tree-no-sema
@@ -103,6 +110,11 @@
 ! HELP-FC1-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-FC1-NEXT: -fno-analyzed-objects-for-unparse
 ! HELP-FC1-NEXT:Do not use the analyzed objects when unparsing
+! HELP-FC1-NEXT: -fno-backslash
+! HELP-FC1-NEXT: -fno-implicit-none
+! HELP-FC1-NEXT: -fno-logical-abbreviations
+! HELP-FC1-NEXT: {{[[:space:]]$}}
+! HELP-FC1-NEXT: -fno-xor-operator
 ! HELP-FC1-NEXT: -fopenacc  Enable OpenACC
 ! HELP-FC1-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
 ! HELP-FC1-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -40,7 +40,12 @@
 ! CHECK-NEXT:Specify where to find the compiled intrinsic modules
 ! CHECK-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! CHECK-NEXT: -flogical-abbreviations Enable logical abbreviations
-! CHECK-NEXT: -fno-color-diagnostics Disable colors in diagnostics
+! CHECK-NEXT: -fno-backslash
+! CHECK-NEXT: -fno-color-diagnostics  Disable colors in diagnostics
+! CHECK-NEXT: -fno-implicit-none
+! CHECK-NEXT: -fno-logical-abbreviations
+! CHECK-NEXT: {{[[:space:]]$}}
+! CHECK-NEXT: -fno-xor-operator
 ! CHECK-NEXT: -fopenacc  Enable OpenACC
 ! CHECK-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
 ! CHECK-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -277,33 +277,27 @@
 }
   }
 
-  if (const llvm::opt::Arg *arg =
-  args.getLastArg(clang::driver::options::OPT_fimplicit_none,
-  clang::driver::options::OPT_fno_implicit_none)) {
-opts.features_.Enable(
-Fortran::common::LanguageFeature::ImplicitNoneTypeAlways,
-arg->getOption().matches(clang::driver::options::OPT_fimplicit_none));
-  }
-  if (const llvm::opt::Arg *arg =
-  args.getLastArg(clang::driver::options::OPT_fbackslash,
-  clang::driver::options::OPT_fno_backslash)) {
-opts.features_.Enable(Fortran::common::LanguageFeature::BackslashEscapes,
-arg->getOption().matches(clang::driver::options::OPT_fbackslash));
-  }
-  if (const llvm::opt::Arg *arg =
-  args.getLastArg(clang::driver::options::OPT_flogical_abbreviations,
-  clang::driver::options::OPT_fno_logical_abbreviations)) {
-opts.features_.Enable(
-Fortran::common::LanguageFeature::LogicalAbbreviations,
-arg->getOption().matches(
-clang::driver::options::OPT_flogical_abbreviations));
-  }
-  if (const llvm::opt::Arg *arg =
-  

[PATCH] D105881: [flang][driver] Switch to `BoolFOption` for boolean options

2021-08-04 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment.

In D105881#2919575 , @jansvoboda11 
wrote:

> The `clang` driver always accepts both the positive and negative options, and 
> it always only considers the argument that appeared last on the command-line 
> (using `ArgList::getLastArg(Pos, Neg)`). This allows users to take a random 
> `clang` driver command-line, append `-feliminate-unused-debug-types` and get 
> the desired behavior, even though the original command-line might have 
> contained `-fno-eliminate-unused-debug-types`.
>
> For the pair of options you mentioned, the `clang` driver knows they are 
> "opt-out" in `clang -cc1`. The behavior is enabled by default in `-cc1` and 
> can be disabled/opted-out through the negative 
> `-fno-eliminate-unused-debug-types`. The positive option is **not** accepted 
> by `clang -cc1`, since explicitly enabling a behavior that's already enabled 
> by default is redundant.
>
> If driver sees `-fno-eliminate-unused-debug-types` as the last option, it 
> will forward it to `clang -cc1`. If it sees `-feliminate-unused-debug-types`, 
> it will ignore it.
>
> Does this make sense?

Yes. Thanks for the context and the rationale! This is much clearer now and 
indeed, makes sense. It's hard to extract this stuff from `Options.td` so I 
really appreciate you taking the time to explain it.

> The `HelpText` doesn't play a role in option being accepted or not in `clang` 
> or `clang -cc1`.

We are 100% in agreement with regard to `HelpText`.

> I think we're in agreement that the current distinction between 
> `HelpText<"">` and no `HelpText` is not particularly useful when printing 
> help.

Yes. Updating printHelp 

 wouldn't be too difficult. Would you be in favor?

> I agree we shouldn't mix Clang and Flang options together in `-help`. But to 
> make sure we're on the same page: this doesn't have to do anything with the 
> presence/absence of `HelpText`, this is controlled by `FlangOption`, 
> `FC1Option`, `CC1Option` etc.

Agreed.

I've experimented with a few more approaches and feel that the cleanest 
approach would be to:

- rename `OptOutFFlag`/`OptInFFlag` as `OptOutCC1FFlag` and `OptInCC1FFlag`
- introduce `OptOutFC1FFlag` and `OptInFC1FFlag`.

We will end-up with a bit of duplication in Options.td, but the long term goal 
is to split it into multiple files anyway. Also, I think that in this case code 
re-use would lead to a rather convoluted implementation. I'm will send an 
updated patch shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105881

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


[PATCH] D105671: [Intrinsics][ObjC] Mark objc_retain and friends as thisreturn.

2021-08-04 Thread Jon Roelofs via Phabricator via cfe-commits
jroelofs added a comment.

In D105671#2884483 , @rjmccall wrote:

> IIUC, we sometimes turn `-retain` message sends into calls to `objc_retain` 
> and so on.  Outside of ARC (the only time they're valid), these calls don't 
> have the semantics of guaranteeing to return their self argument.  So I think 
> this change is only fine if we're not applying it when doing that non-ARC 
> transformation.

I believe this is covered now. The `-retain => objc_retain` transformation 
emits normal function calls, and `objc_retain` calls are only transformed into 
intrinsic calls when ARC is enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105671

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


[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

2021-08-04 Thread Pavel Asyutchenko via Phabricator via cfe-commits
Svenny updated this revision to Diff 364217.
Svenny added a comment.
Herald added a subscriber: dang.

Updated ffile-prefix-map and fmacro-prefix-map in 
clang/include/clang/Driver/Options.td.


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

https://reviews.llvm.org/D107393

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/AST/Expr.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp

Index: clang/test/CodeGenCXX/builtin-source-location.cpp
===
--- clang/test/CodeGenCXX/builtin-source-location.cpp
+++ clang/test/CodeGenCXX/builtin-source-location.cpp
@@ -1,5 +1,13 @@
 // RUN: %clang_cc1 -std=c++2a -fblocks %s -triple x86_64-unknown-unknown -emit-llvm -o %t.ll
 
+// This needs to be performed before #line directives which alter filename
+// RUN: %clang_cc1 -fmacro-prefix-map=%p=/UNLIKELY/PATH -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-PREFIX-MAP
+//
+// CHECK-PREFIX-MAP: /UNLIKELY/PATH{{/|}}builtin-source-location.cpp
+void testRemap() {
+  const char *file = __builtin_FILE();
+}
+
 #line 8 "builtin-source-location.cpp"
 
 struct source_location {
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1460,15 +1460,6 @@
   return TI.getTriple().getEnvironment() == Env.getEnvironment();
 }
 
-static void remapMacroPath(
-SmallString<256> ,
-const std::map>
-) {
-  for (const auto  : MacroPrefixMap)
-if (llvm::sys::path::replace_path_prefix(Path, Entry.first, Entry.second))
-  break;
-}
-
 /// ExpandBuiltinMacro - If an identifier token is read that is to be expanded
 /// as a builtin macro, handle it and return the next token as 'Tok'.
 void Preprocessor::ExpandBuiltinMacro(Token ) {
@@ -1550,7 +1541,7 @@
   } else {
 FN += PLoc.getFilename();
   }
-  remapMacroPath(FN, PPOpts->MacroPrefixMap);
+  getLangOpts().remapPathPrefix(FN);
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3528,6 +3528,9 @@
 GenerateArg(Args, OPT_fexperimental_relative_cxx_abi_vtables, SA);
   else
 GenerateArg(Args, OPT_fno_experimental_relative_cxx_abi_vtables, SA);
+
+  for (const auto  : Opts.MacroPrefixMap)
+GenerateArg(Args, OPT_fmacro_prefix_map_EQ, MP.first + "=" + MP.second, SA);
 }
 
 bool CompilerInvocation::ParseLangArgs(LangOptions , ArgList ,
@@ -4038,6 +4041,12 @@
options::OPT_fno_experimental_relative_cxx_abi_vtables,
TargetCXXABI::usesRelativeVTables(T));
 
+  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ)) {
+auto Split = StringRef(A).split('=');
+Opts.MacroPrefixMap.insert(
+{std::string(Split.first), std::string(Split.second)});
+  }
+
   return Diags.getNumErrors() == NumErrorsBefore;
 }
 
@@ -4110,9 +4119,6 @@
   for (const auto  : Opts.DeserializedPCHDeclsToErrorOn)
 GenerateArg(Args, OPT_error_on_deserialized_pch_decl, D, SA);
 
-  for (const auto  : Opts.MacroPrefixMap)
-GenerateArg(Args, OPT_fmacro_prefix_map_EQ, MP.first + "=" + MP.second, SA);
-
   if (Opts.PrecompiledPreambleBytes != std::make_pair(0u, false))
 GenerateArg(Args, OPT_preamble_bytes_EQ,
 Twine(Opts.PrecompiledPreambleBytes.first) + "," +
@@ -4181,12 +4187,6 @@
   for (const auto *A : Args.filtered(OPT_error_on_deserialized_pch_decl))
 Opts.DeserializedPCHDeclsToErrorOn.insert(A->getValue());
 
-  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ)) {
-auto Split = StringRef(A).split('=');
-Opts.MacroPrefixMap.insert(
-{std::string(Split.first), std::string(Split.second)});
-  }
-
   if (const Arg *A = Args.getLastArg(OPT_preamble_bytes_EQ)) {
 StringRef Value(A->getValue());
 size_t Comma = Value.find(',');
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -186,7 +186,7 @@
   !getModule().getSourceFileName().empty()) {
 std::string Path = getModule().getSourceFileName();
 // Check if a path substitution is needed from the MacroPrefixMap.
-for (const auto  : PPO.MacroPrefixMap)
+for (const auto  : LangOpts.MacroPrefixMap)
   if (Path.rfind(Entry.first, 0) != std::string::npos) {
 Path = Entry.second + Path.substr(Entry.first.size());
 break;
Index: 

[PATCH] D105671: [Intrinsics][ObjC] Mark objc_retain and friends as thisreturn.

2021-08-04 Thread Jon Roelofs via Phabricator via cfe-commits
jroelofs updated this revision to Diff 364215.
jroelofs added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add a note/check to the `[x retain] => objc_retain(x)` test explaining why it 
shouldn't be lowered as an intrinsic call in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105671

Files:
  clang/test/CodeGenObjC/convert-messages-to-runtime-calls.m
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
  llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll

Index: llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll
===
--- llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll
+++ llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll
@@ -6,7 +6,7 @@
 define i8* @test_objc_autorelease(i8* %arg0) {
 ; CHECK-LABEL: test_objc_autorelease
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = notail call i8* @objc_autorelease(i8* %arg0)
+; CHECK-NEXT: %0 = notail call i8* @objc_autorelease(i8* returned %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
   %0 = call i8* @llvm.objc.autorelease(i8* %arg0)
@@ -113,20 +113,31 @@
   ret void
 }
 
+define i8* @test_objc_retain_intrinsic(i8* %arg0) {
+; CHECK-LABEL: test_objc_retain_intrinsic
+; CHECK-NEXT: entry
+; CHECK-NEXT: %0 = tail call i8* @objc_retain(i8* returned %arg0)
+; CHECK-NEXT: ret i8* %0
+entry:
+  %0 = call i8* @llvm.objc.retain(i8* %arg0)
+	ret i8* %0
+}
+
+; Explicit objc_retain calls should not be upgraded to be "thisreturn".
 define i8* @test_objc_retain(i8* %arg0) {
 ; CHECK-LABEL: test_objc_retain
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = tail call i8* @objc_retain(i8* %arg0)
+; CHECK-NEXT: %0 = call i8* @objc_retain(i8* %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
-  %0 = call i8* @llvm.objc.retain(i8* %arg0)
+  %0 = call i8* @objc_retain(i8* %arg0)
 	ret i8* %0
 }
 
 define i8* @test_objc_retainAutorelease(i8* %arg0) {
 ; CHECK-LABEL: test_objc_retainAutorelease
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = call i8* @objc_retainAutorelease(i8* %arg0)
+; CHECK-NEXT: %0 = call i8* @objc_retainAutorelease(i8* returned %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
   %0 = call i8* @llvm.objc.retainAutorelease(i8* %arg0)
@@ -136,7 +147,7 @@
 define i8* @test_objc_retainAutoreleaseReturnValue(i8* %arg0) {
 ; CHECK-LABEL: test_objc_retainAutoreleaseReturnValue
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleaseReturnValue(i8* %arg0)
+; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleaseReturnValue(i8* returned %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
   %0 = tail call i8* @llvm.objc.retainAutoreleaseReturnValue(i8* %arg0)
@@ -146,7 +157,7 @@
 define i8* @test_objc_retainAutoreleasedReturnValue(i8* %arg0) {
 ; CHECK-LABEL: test_objc_retainAutoreleasedReturnValue
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %arg0)
+; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* returned %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
   %0 = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %arg0)
@@ -226,7 +237,7 @@
 define i8* @test_objc_retain_autorelease(i8* %arg0) {
 ; CHECK-LABEL: test_objc_retain_autorelease
 ; CHECK-NEXT: entry
-; CHECK-NEXT: %0 = call i8* @objc_retain_autorelease(i8* %arg0)
+; CHECK-NEXT: %0 = call i8* @objc_retain_autorelease(i8* returned %arg0)
 ; CHECK-NEXT: ret i8* %0
 entry:
   %0 = call i8* @llvm.objc.retain.autorelease(i8* %arg0)
@@ -265,6 +276,7 @@
 declare void @llvm.objc.moveWeak(i8**, i8**)
 declare void @llvm.objc.release(i8*)
 declare i8* @llvm.objc.retain(i8*)
+declare i8* @objc_retain(i8*)
 declare i8* @llvm.objc.retainAutorelease(i8*)
 declare i8* @llvm.objc.retainAutoreleaseReturnValue(i8*)
 declare i8* @llvm.objc.retainAutoreleasedReturnValue(i8*)
@@ -281,6 +293,7 @@
 
 attributes #0 = { nounwind }
 
+; CHECK: declare i8* @objc_retain(i8*) [[NLB:#[0-9]+]]
 ; CHECK: declare i8* @objc_autorelease(i8*)
 ; CHECK: declare void @objc_autoreleasePoolPop(i8*)
 ; CHECK: declare i8* @objc_autoreleasePoolPush()
@@ -291,8 +304,7 @@
 ; CHECK: declare i8* @objc_loadWeak(i8**)
 ; CHECK: declare i8* @objc_loadWeakRetained(i8**)
 ; CHECK: declare void @objc_moveWeak(i8**, i8**)
-; CHECK: declare void @objc_release(i8*) [[NLB:#[0-9]+]]
-; CHECK: declare i8* @objc_retain(i8*) [[NLB]]
+; CHECK: declare void @objc_release(i8*) [[NLB]]
 ; CHECK: declare i8* @objc_retainAutorelease(i8*)
 ; CHECK: declare i8* @objc_retainAutoreleaseReturnValue(i8*)
 ; CHECK: declare i8* @objc_retainAutoreleasedReturnValue(i8*)
Index: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
===
--- llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -110,6 +110,16 @@
 CallInst::TailCallKind TCK = CI->getTailCallKind();
 

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

2021-08-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 364212.
aaron.ballman added a comment.

Updating for some test cases that are still failing.


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

https://reviews.llvm.org/D107402

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/aarch64-outliner.c
  clang/test/Driver/aix-object-mode.c
  clang/test/Driver/amdgpu-invalid-target-id.s
  clang/test/Driver/amdgpu-openmp-system-arch-fail.c
  clang/test/Driver/arm-thumb-only-cores.c
  clang/test/Driver/cl-inputs.c
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/cuda-bad-arch.cu
  clang/test/Driver/cuda-detect.cu
  clang/test/Driver/cuda-omp-unsupported-debug-options.cu
  clang/test/Driver/cuda-options-freebsd.cu
  clang/test/Driver/cuda-options.cu
  clang/test/Driver/cuda-version-check.cu
  clang/test/Driver/defsym.s
  clang/test/Driver/fuse-ld.c
  clang/test/Driver/hip-inputs.hip
  clang/test/Driver/hip-invalid-target-id.hip
  clang/test/Driver/invalid-target-id.cl
  clang/test/Driver/msp430-hwmult.c
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/openmp-offload.c
  clang/test/Driver/rocm-detect.cl
  clang/test/Driver/rocm-detect.hip
  clang/test/Driver/rocm-not-found.cl
  clang/test/Frontend/invalid-cxx-abi.cpp
  clang/test/Frontend/round-trip-cc1-args.c
  clang/test/OpenMP/target_messages.cpp

Index: clang/test/OpenMP/target_messages.cpp
===
--- clang/test/OpenMP/target_messages.cpp
+++ clang/test/OpenMP/target_messages.cpp
@@ -8,12 +8,12 @@
 // CHECK: error: OpenMP target is invalid: 'aaa-bbb-ccc-ddd'
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx64-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
-// CHECK-UNSUPPORTED-HOST-TARGET: error: The target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target.
+// CHECK-UNSUPPORTED-HOST-TARGET: error: target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -fopenmp-targets=hexagon-linux-gnu -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-DEVICE-TARGET %s
 // CHECK-UNSUPPORTED-DEVICE-TARGET: OpenMP target is invalid: 'hexagon-linux-gnu'
 
 // RUN: not %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path .bc -o - 2>&1 | FileCheck --check-prefix NO-HOST-BC %s
-// NO-HOST-BC: The provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found.
+// NO-HOST-BC: provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found
 
 // RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm-bc %s -o %t-ppc-host.bc -DREGION_HOST
 // RUN: not %clang_cc1 -verify=expected,omp4 -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -DREGION_DEVICE 2>&1
Index: clang/test/Frontend/round-trip-cc1-args.c
===
--- clang/test/Frontend/round-trip-cc1-args.c
+++ clang/test/Frontend/round-trip-cc1-args.c
@@ -4,4 +4,4 @@
 
 // CHECK-WITHOUT-ROUND-TRIP-NOT: remark:
 // CHECK-ROUND-TRIP-WITHOUT-REMARKS-NOT: remark:
-// CHECK-ROUND-TRIP-WITH-REMARKS: remark: Generated arguments #{{.*}} in round-trip: {{.*}}
+// CHECK-ROUND-TRIP-WITH-REMARKS: remark: generated arguments #{{.*}} in round-trip: {{.*}}
Index: clang/test/Frontend/invalid-cxx-abi.cpp
===
--- clang/test/Frontend/invalid-cxx-abi.cpp
+++ clang/test/Frontend/invalid-cxx-abi.cpp
@@ -1,8 +1,8 @@
 // These shouldn't be valid -fc++-abi values.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=InvalidABI %s 2>&1 | FileCheck %s -check-prefix=INVALID
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=Fuchsia %s 2>&1 | FileCheck %s -check-prefix=CASE-SENSITIVE
-// INVALID: error: Invalid C++ ABI name 'InvalidABI'
-// CASE-SENSITIVE: error: Invalid C++ ABI name 'Fuchsia'
+// INVALID: error: invalid C++ ABI name 'InvalidABI'
+// CASE-SENSITIVE: error: invalid C++ ABI name 'Fuchsia'
 
 // Some C++ ABIs are not supported on some platforms.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=fuchsia -triple i386 %s 2>&1 | FileCheck %s -check-prefix=UNSUPPORTED-FUCHSIA
Index: clang/test/Driver/rocm-not-found.cl
===
--- 

[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

2021-08-04 Thread Pavel Asyutchenko via Phabricator via cfe-commits
Svenny added a comment.

Great! My commit author string is `Pavel Asyutchenko `.

I have an even uglier hack to work around this: to offset the value of 
`__builtin_FILE()` by compile-time calculated location of some known substring 
like `src` or `include`.


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

https://reviews.llvm.org/D107393

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


[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

2021-08-04 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment.

In D107304#2926310 , @sammccall wrote:

> Clang's current answer is "yes we are GCC >=4, but no we are not GCC >=5".
> This causes the codebase to reject the compiler, because it relies on 
> features/absence of bugs from GCC 5. @ArcSinX any idea what these are, 
> concretely?

The checks about GCC version that I saw were related with some features from 
C++ standard that GCC < 4.X.Y doesn't support and there was a try to implement 
these features for GCC < 4.X.Y, but clang has these features and some kind of 
conflict appears.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107304

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


[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

2021-08-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

clang/include/clang/Driver/Options.td HelpText for -fmacro-prefix-map= needs an 
update.

https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html has more 
documentation


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

https://reviews.llvm.org/D107393

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


[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

2021-08-04 Thread Chris Bowler via Phabricator via cfe-commits
cebowleratibm requested changes to this revision.
cebowleratibm added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Basic/Targets/OSTargets.h:679
 Builder.defineMacro("__TOS_AIX__");
+Builder.defineMacro("__HOS_AIX__");
 

__HOS_AIX__ should only be defined if both the host and the target OS are AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107242

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


[PATCH] D101960: [openmp] Drop requirement on library path environment variables

2021-08-04 Thread Joachim Protze via Phabricator via cfe-commits
protze.joachim added a comment.

To make debugging of failed tests easier, we could just explicitly include `env 
LD_LIBRARY_PATH=...` into each run line - by adding it to the `%libomp-run` 
substitution (and the libomptarget equivalent).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101960

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


[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

2021-08-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

I can push it on your behalf, I want to wait a bit for more comments.
(You need to provide name/email so that you can get the proper attribution `git 
commit --amend --author='...'`)

Looks like a good candidate for release/13.x for build reproducibility.
absl has a macro saying whether `__builtin_FILE` can be used.
I can find a few other places where people do something like 
`-D__builtin_FILE()="file.cc"` probably to work around Clang.


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

https://reviews.llvm.org/D107393

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


[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

2021-08-04 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added a comment.

Can someone file a bug for this and put release-13.0.0 in the blocks field so 
we can track it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74436

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


[PATCH] D106738: [RISCV] Use getNaturalPointeeTypeAlignment to get alignment for stores created for vector builtins.

2021-08-04 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106738

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


[PATCH] D106815: Update: clang/test/Profile/gcc-flag-compatibility.c to have -flto on AIX

2021-08-04 Thread Mark Danial via Phabricator via cfe-commits
madanial marked an inline comment as done.
madanial added a comment.

I need some help to commit this change as I do not have commit access as of now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106815

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


[PATCH] D107492: [clang] Replace asm with __asm__ in cuda header

2021-08-04 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

Implemented by
`F=__clang_cuda_device_functions.h ; sed -i 's$asm$__asm__$g' $F && 
clang-format -i $F`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107492

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


[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

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

Works in Firefox on macOS as well!


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

https://reviews.llvm.org/D107366

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


[PATCH] D107492: [clang] Replace asm with __asm__ in cuda header

2021-08-04 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield created this revision.
JonChesterfield added reviewers: tra, emankov, gtbercea, jdoerfert.
Herald added a subscriber: yaxunl.
JonChesterfield requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Asm is a gnu extension for C, so at present -fopenmp -std=c99
and similar fail to compile on nvptx, bug 51344

Changing to `__asm__` or `__asm` works for openmp, all three appear to work
for cuda. Suggesting `__asm__` here as `__asm` is used by MSVC with different
syntax, so this should make for better error diagnostics if the header is
passed to a compiler other than clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107492

Files:
  clang/lib/Headers/__clang_cuda_device_functions.h

Index: clang/lib/Headers/__clang_cuda_device_functions.h
===
--- clang/lib/Headers/__clang_cuda_device_functions.h
+++ clang/lib/Headers/__clang_cuda_device_functions.h
@@ -34,10 +34,12 @@
   return __nv_brevll(__a);
 }
 #if defined(__cplusplus)
-__DEVICE__ void __brkpt() { asm volatile("brkpt;"); }
+__DEVICE__ void __brkpt() { __asm__ volatile("brkpt;"); }
 __DEVICE__ void __brkpt(int __a) { __brkpt(); }
 #else
-__DEVICE__ void __attribute__((overloadable)) __brkpt(void) { asm volatile("brkpt;"); }
+__DEVICE__ void __attribute__((overloadable)) __brkpt(void) {
+  __asm__ volatile("brkpt;");
+}
 __DEVICE__ void __attribute__((overloadable)) __brkpt(int __a) { __brkpt(); }
 #endif
 __DEVICE__ unsigned int __byte_perm(unsigned int __a, unsigned int __b,
@@ -507,7 +509,7 @@
 }
 
 // Parameter must have a known integer value.
-#define __prof_trigger(__a) asm __volatile__("pmevent \t%0;" ::"i"(__a))
+#define __prof_trigger(__a) __asm__ __volatile__("pmevent \t%0;" ::"i"(__a))
 __DEVICE__ int __rhadd(int __a, int __b) { return __nv_rhadd(__a, __b); }
 __DEVICE__ unsigned int __sad(int __a, int __b, unsigned int __c) {
   return __nv_sad(__a, __b, __c);
@@ -526,7 +528,7 @@
 __DEVICE__ void __threadfence(void) { __nvvm_membar_gl(); }
 __DEVICE__ void __threadfence_block(void) { __nvvm_membar_cta(); };
 __DEVICE__ void __threadfence_system(void) { __nvvm_membar_sys(); };
-__DEVICE__ void __trap(void) { asm volatile("trap;"); }
+__DEVICE__ void __trap(void) { __asm__ volatile("trap;"); }
 __DEVICE__ unsigned int __uAtomicAdd(unsigned int *__p, unsigned int __v) {
   return __nvvm_atom_add_gen_i((int *)__p, __v);
 }
@@ -1038,7 +1040,7 @@
 __DEVICE__ unsigned int __vsubus4(unsigned int __a, unsigned int __b) {
   return __nv_vsubus4(__a, __b);
 }
-#else // CUDA_VERSION >= 9020
+#else  // CUDA_VERSION >= 9020
 // CUDA no longer provides inline assembly (or bitcode) implementation of these
 // functions, so we have to reimplment them. The implementation is naive and is
 // not optimized for performance.
@@ -1051,122 +1053,136 @@
 }
 __DEVICE__ unsigned int __vabs2(unsigned int __a) {
   unsigned int r;
-  asm("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(0), "r"(0));
+  __asm__("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabs4(unsigned int __a) {
   unsigned int r;
-  asm("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(0), "r"(0));
+  __asm__("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(0), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsdiffs2(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff2.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 
 __DEVICE__ unsigned int __vabsdiffs4(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff4.s32.s32.s32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsdiffu2(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff2.u32.u32.u32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff2.u32.u32.u32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsdiffu4(unsigned int __a, unsigned int __b) {
   unsigned int r;
-  asm("vabsdiff4.u32.u32.u32 %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(__b), "r"(0));
+  __asm__("vabsdiff4.u32.u32.u32 %0,%1,%2,%3;"
+  : "=r"(r)
+  : "r"(__a), "r"(__b), "r"(0));
   return r;
 }
 __DEVICE__ unsigned int __vabsss2(unsigned int __a) {
   unsigned int r;
-  asm("vabsdiff2.s32.s32.s32.sat %0,%1,%2,%3;"
-  : "=r"(r)
-  : "r"(__a), "r"(0), "r"(0));
+  __asm__("vabsdiff2.s32.s32.s32.sat 

[PATCH] D105821: [analyzer] [WIP] Model destructor for std::unique_ptr

2021-08-04 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:298
+  }
+  return {State, true};
+}

Something's not right. Returning `true` here would discard the state and 
terminate `evalCall` as failed. Why compute the invalidated state if we throw 
it away?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105821

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


[PATCH] D99260: [analyzer] Fix false positives in inner pointer checker (PR49628)

2021-08-04 Thread Ali Shuja Siddiqui via Phabricator via cfe-commits
alishuja added a comment.
Herald added a subscriber: manas.

Based on the comment from @vsavchenko , I had made an addition for a check for 
`__addressof` alongside `addressof` in the checker. What would be the correct 
way of pushing the diff here? Should I reopen this revision or use the update 
diff button on the top? I apologize, I'm completely new to pushing changes to 
LLVM.

Thanks,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99260

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


[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

2021-08-04 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

I disagree that this is a pointless discussion, FWIW.

The `__GNUC__` macro is used for compiler feature (or bug?) detection.
Clang's current answer is "yes we are GCC >=4, but no we are not GCC >=5".
This causes the codebase to reject the compiler, because it relies on 
features/absence of bugs from GCC 5. @ArcSinX any idea what these are, 
concretely?
If clang has those features/lacks those bugs, there's some value in making it 
pass the feature check.

Problem is, there are significant extensions in GCC 4.3 that clang doesn't 
implement. Per user's manual :

> clang does not support `__builtin_va_arg_pack`/`__builtin_va_arg_pack_len`. 
> This is used rarely, but in some potentially interesting places, like the 
> glibc headers, so it may be implemented pending user demand. Note that 
> because clang pretends to be like GCC 4.2, and this extension was introduced 
> in 4.3, the glibc headers will not try to use this extension with clang at 
> the moment.

See also this thread 
 and D51011 
. So we can't simply bump clang's compatibilty 
claims.

---

For clangd, I think this illustrates why implicitly increasing `__GNUC__` (e.g. 
via query-driver) isn't safe in general.
For files that **optionally** use e.g. `__builtin_va_arg_pack` (like glibc 
headers), we'll direct the preprocessor to take that path and then reject the 
code.

With codebases that refuse to build at all without large `__GNUC__` but 
actually mostly parse fine, this is unfortunate, but the sort of thing that 
users probably should opt into.
The symptom `#error You need GCC X.Y to build this code` is much easier to 
understand than the results of trying to use unsupported extensions.
I think the best we can do here is document the workaround.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107304

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


[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

2021-08-04 Thread Pavel Asyutchenko via Phabricator via cfe-commits
Svenny marked an inline comment as done.
Svenny added a comment.

Could you please land it? I don't have commit access.

Also, any chance this patch can get into 13.x release branch? Not sure if it 
counts as a feature or a bugfix :)


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

https://reviews.llvm.org/D107393

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


[PATCH] D106030: [Clang] add support for error+warning fn attrs

2021-08-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:6057
+assertions that depend on optimizations, while providing diagnostics
+pointing to precise locations of the call site in the source.
+  }];

@aaron.ballman anything I should add to the documentation blurb?



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1205
 def BackendOptimizationFailure : DiagGroup<"pass-failed">;
+def BackendUserDiagnostic : DiagGroup<"user-diagnostic">;
 

aaron.ballman wrote:
> nickdesaulniers wrote:
> > aaron.ballman wrote:
> > > nickdesaulniers wrote:
> > > > aaron.ballman wrote:
> > > > > GCC doesn't seem to support this spelling -- do they have a different 
> > > > > one we should reuse?
> > > > I think these diagnostics don't have corresponding flags in GCC, so 
> > > > they cannot be disabled.
> > > > 
> > > > Without adding this, clang/test/Misc/warning-flags.c would fail, 
> > > > because I was adding a new unnamed warning 
> > > > `warn_fe_backend_user_diagnostic`.  Perhaps I should not be adding this 
> > > > line here, and doing something else?
> > > > 
> > > > That test says we shouldn't be adding new warnings not controlled by 
> > > > flags, but that's very much my intention.
> > > > 
> > > > Though now `-Wno-user-diagnostic` doesn't produce a 
> > > > `-Wunknown-warning-option` diagnostic...
> > > Ah! I think the warning attribute should be controllable via a diagnostic 
> > > flag (we should always be able to disable warnings with some sort of 
> > > flag) and I don't think the error attribute should be controllable (an 
> > > error is an error and should stop translation, same as with `#error`).
> > > 
> > > Normally, I'd say "let's use the same flag that controls `#warning`" 
> > > but...
> > > ```
> > > def PoundWarning : DiagGroup<"#warnings">;
> > > ```
> > > That's... not exactly an obvious flag for the warning attribute. So I 
> > > would probably name it `warning-attributes` similar to how we have 
> > > `deprecated-attributes` already.
> > Done, now `-Wno-warning-attributes` doesn't produce 
> > `-Wunknown-warning-option`, but also doesn't disable the diagnostic.  Was 
> > there something else I needed to add?
> huh, that sounds suspicious -- you don't need to do anything special for 
> `-Wno-foo` handling, that should be automatically supported via tablegen. I'm 
> not certain why you're not seeing `-Wno-warning-attributes` silencing the 
> warnings.
Ah! Because I was testing `__attribute__((error("")))` not 
`__attribute__((warning("")))`. `-Wno-warning-attributes` only affects the 
latter, not the former.  I should add a test for `-Wno-warning-attributes`! Is 
this something I also need to add documentation for?



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:783
+  else
+Diags.Report(DiagID) << CalleeName << D.getMsgStr();
+}

aaron.ballman wrote:
> nickdesaulniers wrote:
> > aaron.ballman wrote:
> > > nickdesaulniers wrote:
> > > > nickdesaulniers wrote:
> > > > > aaron.ballman wrote:
> > > > > > nickdesaulniers wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > I think the usability in this situation is a concern -- only 
> > > > > > > > having a function name but no source location information is 
> > > > > > > > pretty tricky. Do you have a sense for how often we would hit 
> > > > > > > > this case?
> > > > > > > I don't think we ever encounter it when building the kernel 
> > > > > > > sources. Perhaps I should make this an assertion on 
> > > > > > > `LocCookie.isValid()` instead?
> > > > > > > 
> > > > > > > While clang should always attach a `srcloc` MetaData node, other 
> > > > > > > frontend might not, so it's optional.  But this TU is strictly 
> > > > > > > Clang.
> > > > > > I thought backend optimization passes would drop source location 
> > > > > > information with some regularity, so I'm a bit hesitant to go with 
> > > > > > an assertion unless it turns out I'm wrong there. However, if the 
> > > > > > backend shouldn't ever lose *this* source location information, 
> > > > > > then I think it's good to make it an assertion.
> > > > > Perhaps debug info, but this SourceLocation is squirreled away in a 
> > > > > Metadata node attached to the call site.
> > > > > 
> > > > > Ah, but via further testing of @arsenm 's point ("what about indirect 
> > > > > calls?"), this case is tricky:
> > > > > ```
> > > > > // clang -O2 foo.c
> > > > > __attribute__((error("oh no foo")))
> > > > > void foo(void);
> > > > > 
> > > > > void (*bar)(void);
> > > > > 
> > > > > void baz(void) {
> > > > >   bar = foo;
> > > > >   bar();
> > > > > }
> > > > > ```
> > > > > since we did not emit the Metadata node on the call, but then later 
> > > > > during optimizations we replaced the call to `bar` with a call to 
> > > > > `foo`. At that point, the front end did not emit a Metadata node with 
> > > > > source location information, 

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

2021-08-04 Thread Steven Wan via Phabricator via cfe-commits
stevewan marked an inline comment as done.
stevewan added inline comments.



Comment at: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp:11
+
+// CHECK: @c = global %struct.C zeroinitializer, align 2

sfertile wrote:
> Minor nit: I think the other test can live in this file, highlighting the 
> difference between the 2 cases is nice. Also I think we should add a typedef 
> test in here as well.
Merged the tests as suggested. We have the typedef coverage in 
`aix-power-alignment-typedef.cpp` and `aix-power-alignment-typedef2.cpp`, not 
sure if want to merge those as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

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


[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

2021-08-04 Thread Pavel Asyutchenko via Phabricator via cfe-commits
Svenny updated this revision to Diff 364195.
Svenny added a comment.

Moved new test to CodeGenCXX/builtin-source-location.cpp.


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

https://reviews.llvm.org/D107393

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/AST/Expr.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp

Index: clang/test/CodeGenCXX/builtin-source-location.cpp
===
--- clang/test/CodeGenCXX/builtin-source-location.cpp
+++ clang/test/CodeGenCXX/builtin-source-location.cpp
@@ -1,5 +1,13 @@
 // RUN: %clang_cc1 -std=c++2a -fblocks %s -triple x86_64-unknown-unknown -emit-llvm -o %t.ll
 
+// This needs to be performed before #line directives which alter filename
+// RUN: %clang_cc1 -fmacro-prefix-map=%p=/UNLIKELY/PATH -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-PREFIX-MAP
+//
+// CHECK-PREFIX-MAP: /UNLIKELY/PATH{{/|}}builtin-source-location.cpp
+void testRemap() {
+  const char *file = __builtin_FILE();
+}
+
 #line 8 "builtin-source-location.cpp"
 
 struct source_location {
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1460,15 +1460,6 @@
   return TI.getTriple().getEnvironment() == Env.getEnvironment();
 }
 
-static void remapMacroPath(
-SmallString<256> ,
-const std::map>
-) {
-  for (const auto  : MacroPrefixMap)
-if (llvm::sys::path::replace_path_prefix(Path, Entry.first, Entry.second))
-  break;
-}
-
 /// ExpandBuiltinMacro - If an identifier token is read that is to be expanded
 /// as a builtin macro, handle it and return the next token as 'Tok'.
 void Preprocessor::ExpandBuiltinMacro(Token ) {
@@ -1550,7 +1541,7 @@
   } else {
 FN += PLoc.getFilename();
   }
-  remapMacroPath(FN, PPOpts->MacroPrefixMap);
+  getLangOpts().remapPathPrefix(FN);
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3528,6 +3528,9 @@
 GenerateArg(Args, OPT_fexperimental_relative_cxx_abi_vtables, SA);
   else
 GenerateArg(Args, OPT_fno_experimental_relative_cxx_abi_vtables, SA);
+
+  for (const auto  : Opts.MacroPrefixMap)
+GenerateArg(Args, OPT_fmacro_prefix_map_EQ, MP.first + "=" + MP.second, SA);
 }
 
 bool CompilerInvocation::ParseLangArgs(LangOptions , ArgList ,
@@ -4038,6 +4041,12 @@
options::OPT_fno_experimental_relative_cxx_abi_vtables,
TargetCXXABI::usesRelativeVTables(T));
 
+  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ)) {
+auto Split = StringRef(A).split('=');
+Opts.MacroPrefixMap.insert(
+{std::string(Split.first), std::string(Split.second)});
+  }
+
   return Diags.getNumErrors() == NumErrorsBefore;
 }
 
@@ -4110,9 +4119,6 @@
   for (const auto  : Opts.DeserializedPCHDeclsToErrorOn)
 GenerateArg(Args, OPT_error_on_deserialized_pch_decl, D, SA);
 
-  for (const auto  : Opts.MacroPrefixMap)
-GenerateArg(Args, OPT_fmacro_prefix_map_EQ, MP.first + "=" + MP.second, SA);
-
   if (Opts.PrecompiledPreambleBytes != std::make_pair(0u, false))
 GenerateArg(Args, OPT_preamble_bytes_EQ,
 Twine(Opts.PrecompiledPreambleBytes.first) + "," +
@@ -4181,12 +4187,6 @@
   for (const auto *A : Args.filtered(OPT_error_on_deserialized_pch_decl))
 Opts.DeserializedPCHDeclsToErrorOn.insert(A->getValue());
 
-  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ)) {
-auto Split = StringRef(A).split('=');
-Opts.MacroPrefixMap.insert(
-{std::string(Split.first), std::string(Split.second)});
-  }
-
   if (const Arg *A = Args.getLastArg(OPT_preamble_bytes_EQ)) {
 StringRef Value(A->getValue());
 size_t Comma = Value.find(',');
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -186,7 +186,7 @@
   !getModule().getSourceFileName().empty()) {
 std::string Path = getModule().getSourceFileName();
 // Check if a path substitution is needed from the MacroPrefixMap.
-for (const auto  : PPO.MacroPrefixMap)
+for (const auto  : LangOpts.MacroPrefixMap)
   if (Path.rfind(Entry.first, 0) != std::string::npos) {
 Path = Entry.second + Path.substr(Entry.first.size());
 break;
Index: clang/lib/Basic/LangOptions.cpp
===
--- 

[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

2021-08-04 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment.

This is technically in the "Waiting on Review" queue, that's why I also added 
another reviewer :). The original revision got accepted and then I reverted and 
updated the patch/test to honor the `Deserialize` value (which was broken in 
the first version). I guess the fact that this got reviewed, landed, reverted 
and back into review caused some confusion

Also obligatory "friendly ping" on this :)


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

https://reviews.llvm.org/D80878

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


[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

2021-08-04 Thread Vyacheslav Zakharin via Phabricator via cfe-commits
vzakhari updated this revision to Diff 364193.

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

https://reviews.llvm.org/D99551

Files:
  clang/test/Driver/Inputs/empty-elf-template.yaml
  clang/test/Driver/clang-offload-wrapper.c
  clang/tools/clang-offload-wrapper/CMakeLists.txt
  clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp

Index: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
===
--- clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
+++ clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
@@ -17,27 +17,37 @@
 #include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/EndianStream.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/VCSRevision.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 
+#define OPENMP_OFFLOAD_IMAGE_VERSION "1.0"
+
 using namespace llvm;
+using namespace llvm::object;
 
 static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
@@ -60,6 +70,12 @@
cl::desc("Target triple for the output module"),
cl::value_desc("triple"), cl::cat(ClangOffloadWrapperCategory));
 
+static cl::opt SaveTemps(
+"save-temps",
+cl::desc("Save temporary files that may be produced by the tool. "
+ "This option forces print-out of the temporary files' names."),
+cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -70,6 +86,15 @@
   StructType *ImageTy = nullptr;
   StructType *DescTy = nullptr;
 
+  std::string ToolName;
+  std::string ObjcopyPath;
+  // Temporary file names that may be created during adding notes
+  // to ELF offload images. Use -save-temps to keep them and also
+  // see their names. A temporary file's name includes the name
+  // of the original input ELF image, so you can easily match
+  // them, if you have multiple inputs.
+  std::vector TempFiles;
+
 private:
   IntegerType *getSizeTTy() {
 switch (M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C))) {
@@ -294,8 +319,61 @@
   }
 
 public:
-  BinaryWrapper(StringRef Target) : M("offload.wrapper.object", C) {
+  BinaryWrapper(StringRef Target, StringRef ToolName)
+  : M("offload.wrapper.object", C), ToolName(ToolName) {
 M.setTargetTriple(Target);
+// Look for llvm-objcopy in the same directory, from which
+// clang-offload-wrapper is invoked. This helps OpenMP offload
+// LIT tests.
+
+// This just needs to be some symbol in the binary; C++ doesn't
+// allow taking the address of ::main however.
+void *P = (void *)(intptr_t)
+std::string COWPath = sys::fs::getMainExecutable(ToolName.str().c_str(), P);
+if (!COWPath.empty()) {
+  auto COWDir = sys::path::parent_path(COWPath);
+  ErrorOr ObjcopyPathOrErr =
+  sys::findProgramByName("llvm-objcopy", {COWDir});
+  if (ObjcopyPathOrErr) {
+ObjcopyPath = *ObjcopyPathOrErr;
+return;
+  }
+
+  // Otherwise, look through PATH environment.
+}
+
+ErrorOr ObjcopyPathOrErr =
+sys::findProgramByName("llvm-objcopy");
+if (!ObjcopyPathOrErr) {
+  WithColor::warning(errs(), ToolName)
+  << "cannot find llvm-objcopy[.exe] in PATH; ELF notes cannot be "
+ "added.\n";
+  return;
+}
+
+ObjcopyPath = *ObjcopyPathOrErr;
+  }
+
+  ~BinaryWrapper() {
+if (TempFiles.empty())
+  return;
+
+StringRef ToolNameRef(ToolName);
+auto warningOS = [ToolNameRef]() -> raw_ostream & {
+  return WithColor::warning(errs(), ToolNameRef);
+};
+
+for (auto  : TempFiles) {
+  if (SaveTemps) {
+warningOS() << "keeping temporary file " << F << "\n";
+continue;
+  }
+
+  auto EC = sys::fs::remove(F, false);
+  if (EC)
+warningOS() << "cannot remove temporary file " << F << ": "
+<< EC.message().c_str() << "\n";
+}
   }
 
   const Module (ArrayRef> Binaries) {
@@ -305,6 +383,205 @@
 createUnregisterFunction(Desc);
 return M;
   }
+
+  std::unique_ptr addELFNotes(std::unique_ptr Buf,
+StringRef OriginalFileName) {
+// Cannot add notes, if llvm-objcopy is not 

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for FP atomics in GFX90A

2021-08-04 Thread Stanislav Mekhanoshin via Phabricator via cfe-commits
rampitec added a comment.

The title should not mention gfx90a, it is not true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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


[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for FP atomics in GFX90A

2021-08-04 Thread Stanislav Mekhanoshin via Phabricator via cfe-commits
rampitec added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12139
+OptimizationRemark Remark(DEBUG_TYPE, "Passed", RMW->getFunction());
+Remark << "A hardware instruction was generated";
+return Remark;

gandhi21299 wrote:
> rampitec wrote:
> > It was not generated. We have multiple returns below this point. Some of 
> > them return None and some CmpXChg for various reasons. The request was to 
> > report when we produce the instruction *if* it is unsafe, not just that we 
> > are about to produce an instruction.
> > 
> > Then to make it useful a remark should tell what was the reason to either 
> > produce an instruction or expand it. Looking at a stream of remarks in a 
> > big program one would also want to understand what exactly was expanded and 
> > what was left as is. A stream of messages "A hardware instruction was 
> > generated" unlikely will help to understand what was done.
> Will the hardware instruction be generated in the end of this function then?
It will not be generated here to begin with. If the function returns None the 
atomicrmw will be just left as is and then later selected into the instruction. 
But if you read the function, it has many returns for many different reasons, 
and that is exactly what a useful remark shall report.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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


[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for FP atomics in GFX90A

2021-08-04 Thread Anshil Gandhi via Phabricator via cfe-commits
gandhi21299 added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12139
+OptimizationRemark Remark(DEBUG_TYPE, "Passed", RMW->getFunction());
+Remark << "A hardware instruction was generated";
+return Remark;

rampitec wrote:
> It was not generated. We have multiple returns below this point. Some of them 
> return None and some CmpXChg for various reasons. The request was to report 
> when we produce the instruction *if* it is unsafe, not just that we are about 
> to produce an instruction.
> 
> Then to make it useful a remark should tell what was the reason to either 
> produce an instruction or expand it. Looking at a stream of remarks in a big 
> program one would also want to understand what exactly was expanded and what 
> was left as is. A stream of messages "A hardware instruction was generated" 
> unlikely will help to understand what was done.
Will the hardware instruction be generated in the end of this function then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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


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

2021-08-04 Thread Steven Wan via Phabricator via cfe-commits
stevewan updated this revision to Diff 364186.
stevewan added a comment.

Merge tests into one file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/Layout/aix-alignof-align-and-pack-attr.cpp


Index: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | 
\
+// RUN:   FileCheck %s
+
+namespace test1 {
+struct __attribute__((__aligned__(2))) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test1{{.*}}c{{.*}} = global %"struct.test1::C" 
zeroinitializer, align 8
+} // namespace test1
+
+namespace test2 {
+struct __attribute__((__aligned__(2), packed)) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test2{{.*}}c{{.*}} = global %"struct.test2::C" 
zeroinitializer, align 2
+} // namespace test2
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2478,11 +2478,16 @@
 return ABIAlign;
 
   if (const auto *RT = T->getAs()) {
-if (TI.AlignIsRequired || RT->getDecl()->isInvalidDecl())
+const RecordDecl *RD = RT->getDecl();
+
+// When used as part of a typedef, or together with a 'packed' attribute,
+// the 'aligned' attribute can be used to decrease alignment.
+if ((TI.AlignIsRequired && T->getAs() != nullptr) ||
+RD->isInvalidDecl())
   return ABIAlign;
 
 unsigned PreferredAlign = static_cast(
-toBits(getASTRecordLayout(RT->getDecl()).PreferredAlignment));
+toBits(getASTRecordLayout(RD).PreferredAlignment));
 assert(PreferredAlign >= ABIAlign &&
"PreferredAlign should be at least as large as ABIAlign.");
 return PreferredAlign;


Index: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+namespace test1 {
+struct __attribute__((__aligned__(2))) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test1{{.*}}c{{.*}} = global %"struct.test1::C" zeroinitializer, align 8
+} // namespace test1
+
+namespace test2 {
+struct __attribute__((__aligned__(2), packed)) C {
+  double x;
+} c;
+
+// CHECK: @{{.*}}test2{{.*}}c{{.*}} = global %"struct.test2::C" zeroinitializer, align 2
+} // namespace test2
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2478,11 +2478,16 @@
 return ABIAlign;
 
   if (const auto *RT = T->getAs()) {
-if (TI.AlignIsRequired || RT->getDecl()->isInvalidDecl())
+const RecordDecl *RD = RT->getDecl();
+
+// When used as part of a typedef, or together with a 'packed' attribute,
+// the 'aligned' attribute can be used to decrease alignment.
+if ((TI.AlignIsRequired && T->getAs() != nullptr) ||
+RD->isInvalidDecl())
   return ABIAlign;
 
 unsigned PreferredAlign = static_cast(
-toBits(getASTRecordLayout(RT->getDecl()).PreferredAlignment));
+toBits(getASTRecordLayout(RD).PreferredAlignment));
 assert(PreferredAlign >= ABIAlign &&
"PreferredAlign should be at least as large as ABIAlign.");
 return PreferredAlign;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

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

Thanks!




Comment at: clang/test/CodeGen/macro-prefix-map.c:1
+// RUN: %clang_cc1 -fmacro-prefix-map=%p=/UNLIKELY/PATH -emit-llvm -o - %s | 
FileCheck %s
+

The test may be placed in CodeGenCXX/builtin-source-location.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107393

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


[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

2021-08-04 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

@teemperor is there a reason why the fixed patch never landed? This looks a 
good improvement to have.


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

https://reviews.llvm.org/D80878

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


[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

2021-08-04 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment.

In D107304#2924886 , @kadircet wrote:

> Ok if you think that setting GCC version to higher values by default won't 
> break anything,

I think that GCC version should be the same, which was used to build the 
project: it can be higher or lower than 4.2.1

In D107304#2925220 , @joerg wrote:

> This discussion is becoming pointless.

Yes, seems we can't get agreement here, so I am planning to abandon this.

> Clang is not a 1:1 replacement for later GCC versions. Easiest example is the 
> support for `__builtin_apply` and friends.

Still we have no example when GCC x.y.z can compile something, but clang with 
-fgnuc-version=x.y.z can't process this project with some critical errors (but 
works ok with -fgnuc-version=4.2.1). But it's easy to create an example when 
-fgnuc-version=4.2.1 doesn't work (example in the description).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107304

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


[PATCH] D106960: [OffloadArch] Library to query properties of current offload archicture

2021-08-04 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D106960#2925610 , @ye-luo wrote:

> my second GPU is NVIDIA 3060Ti (sm_86)
> I build my app daily with -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_80.
>
> About sm_80 binary able ot run on sm_86
> https://docs.nvidia.com/cuda/ampere-compatibility-guide/index.html#application-compatibility-on-ampere

Keep in mind that the binaries compiled for sm_80 will likely run a lot slower 
on sm_86. sm_86 has distinctly different hardware and the code generated for 
sm_80 will be sub-optimal for it.
I don't have the Ampere cards to compare, but sm_70 binaries running on sm_75 
were reached only about 1/2 of the speed of the same code compiled for sm_75 
when it was operating on fp16.

NVIDIA didn't provide performance tuning guide for Ampere, but here's what it 
had to say about Volta/Turing:
https://docs.nvidia.com/cuda/turing-tuning-guide/index.html#tensor-operations

> Any binary compiled for Volta will run on Turing, but Volta binaries using 
> Tensor Cores will only be able to reach half of Turing's Tensor Core peak 
> performance. 
> Recompiling the binary specifically for Turing would allow it to reach the 
> peak performance.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106960

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


[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

2021-08-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 364172.
aaron.ballman added a comment.

Updating based on review feedback.


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

https://reviews.llvm.org/D107402

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/aarch64-outliner.c
  clang/test/Driver/aix-object-mode.c
  clang/test/Driver/amdgpu-invalid-target-id.s
  clang/test/Driver/amdgpu-openmp-system-arch-fail.c
  clang/test/Driver/arm-thumb-only-cores.c
  clang/test/Driver/cl-inputs.c
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/cuda-bad-arch.cu
  clang/test/Driver/cuda-detect.cu
  clang/test/Driver/cuda-omp-unsupported-debug-options.cu
  clang/test/Driver/cuda-options-freebsd.cu
  clang/test/Driver/cuda-options.cu
  clang/test/Driver/cuda-version-check.cu
  clang/test/Driver/defsym.s
  clang/test/Driver/fuse-ld.c
  clang/test/Driver/hip-inputs.hip
  clang/test/Driver/hip-invalid-target-id.hip
  clang/test/Driver/invalid-target-id.cl
  clang/test/Driver/msp430-hwmult.c
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/openmp-offload.c
  clang/test/Driver/rocm-detect.cl
  clang/test/Driver/rocm-detect.hip
  clang/test/Driver/rocm-not-found.cl
  clang/test/Frontend/invalid-cxx-abi.cpp
  clang/test/Frontend/round-trip-cc1-args.c
  clang/test/OpenMP/target_messages.cpp

Index: clang/test/OpenMP/target_messages.cpp
===
--- clang/test/OpenMP/target_messages.cpp
+++ clang/test/OpenMP/target_messages.cpp
@@ -8,12 +8,12 @@
 // CHECK: error: OpenMP target is invalid: 'aaa-bbb-ccc-ddd'
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx64-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -triple nvptx-nvidia-cuda -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-HOST-TARGET %s
-// CHECK-UNSUPPORTED-HOST-TARGET: error: The target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target.
+// CHECK-UNSUPPORTED-HOST-TARGET: error: target '{{nvptx64-nvidia-cuda|nvptx-nvidia-cuda}}' is not a supported OpenMP host target
 // RUN: not %clang_cc1 -fopenmp -std=c++11 -fopenmp-targets=hexagon-linux-gnu -o - %s 2>&1 | FileCheck --check-prefix CHECK-UNSUPPORTED-DEVICE-TARGET %s
 // CHECK-UNSUPPORTED-DEVICE-TARGET: OpenMP target is invalid: 'hexagon-linux-gnu'
 
 // RUN: not %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path .bc -o - 2>&1 | FileCheck --check-prefix NO-HOST-BC %s
-// NO-HOST-BC: The provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found.
+// NO-HOST-BC: provided host compiler IR file '.bc' is required to generate code for OpenMP target regions but cannot be found
 
 // RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm-bc %s -o %t-ppc-host.bc -DREGION_HOST
 // RUN: not %clang_cc1 -verify=expected,omp4 -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -DREGION_DEVICE 2>&1
Index: clang/test/Frontend/round-trip-cc1-args.c
===
--- clang/test/Frontend/round-trip-cc1-args.c
+++ clang/test/Frontend/round-trip-cc1-args.c
@@ -4,4 +4,4 @@
 
 // CHECK-WITHOUT-ROUND-TRIP-NOT: remark:
 // CHECK-ROUND-TRIP-WITHOUT-REMARKS-NOT: remark:
-// CHECK-ROUND-TRIP-WITH-REMARKS: remark: Generated arguments #{{.*}} in round-trip: {{.*}}
+// CHECK-ROUND-TRIP-WITH-REMARKS: remark: generated arguments #{{.*}} in round-trip: {{.*}}
Index: clang/test/Frontend/invalid-cxx-abi.cpp
===
--- clang/test/Frontend/invalid-cxx-abi.cpp
+++ clang/test/Frontend/invalid-cxx-abi.cpp
@@ -1,8 +1,8 @@
 // These shouldn't be valid -fc++-abi values.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=InvalidABI %s 2>&1 | FileCheck %s -check-prefix=INVALID
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=Fuchsia %s 2>&1 | FileCheck %s -check-prefix=CASE-SENSITIVE
-// INVALID: error: Invalid C++ ABI name 'InvalidABI'
-// CASE-SENSITIVE: error: Invalid C++ ABI name 'Fuchsia'
+// INVALID: error: invalid C++ ABI name 'InvalidABI'
+// CASE-SENSITIVE: error: invalid C++ ABI name 'Fuchsia'
 
 // Some C++ ABIs are not supported on some platforms.
 // RUN: not %clang_cc1 -S -emit-llvm -o /dev/null -fc++-abi=fuchsia -triple i386 %s 2>&1 | FileCheck %s -check-prefix=UNSUPPORTED-FUCHSIA
Index: clang/test/Driver/rocm-not-found.cl
===
--- 

[PATCH] D106909: [clang] Add clang builtins support for gfx90a

2021-08-04 Thread Stanislav Mekhanoshin via Phabricator via cfe-commits
rampitec added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsAMDGPU.def:201
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_f32, "ff*1f", "t", 
"gfx90a-insts")
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_v2f16, "V2hV2h*1V2h", "t", 
"gfx90a-insts")
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fmin_f64, "dd*1d", "t", 
"gfx90a-insts")

gandhi21299 wrote:
> I tried add target feature gfx908-insts for this builtin but the frontend 
> complains that it should have target feature gfx90a-insts.
That was for global_atomic_fadd_f32, but as per discussion we are going to use 
builtin only starting from gfx90a because of the noret problem. Comments in the 
review are off their positions after multiple patch updates.



Comment at: clang/include/clang/Basic/BuiltinsAMDGPU.def:210
+TARGET_BUILTIN(__builtin_amdgcn_ds_atomic_fadd_f64, "dd*3d", "t", 
"gfx90a-insts")
+TARGET_BUILTIN(__builtin_amdgcn_ds_atomic_fadd_f32, "ff*3f", "t", "gfx8-insts")
+

This needs tests with a gfx8 target and a negative test with gfx7.



Comment at: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl:13
+// GFX90A:  global_atomic_add_f64
+void test_global_add(__global double *addr, double x) {
+  double *rtn;

Use _f64 or _double in the test name.



Comment at: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl:31
+// GFX90A:  global_atomic_min_f64
+void test_global_global_min(__global double *addr, double x){
+  double *rtn;

Same here and in other double tests, use a suffix f64 or double.



Comment at: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl:67
+// GFX90A:  flat_atomic_min_f64
+void test_flat_min_constant(__generic double *addr, double x){
+  double *rtn;

'constant' is wrong. It is flat. Here and everywhere.



Comment at: clang/test/CodeGenOpenCL/builtins-fp-atomics-unsupported-gfx908.cl:7
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f64(addr, x); // 
expected-error{{'__builtin_amdgcn_global_atomic_fadd_f64' needs target feature 
gfx90a-insts}}
+}

Need to check all other buintins too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106909

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


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

2021-08-04 Thread Sean Fertile via Phabricator via cfe-commits
sfertile added inline comments.



Comment at: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp:11
+
+// CHECK: @c = global %struct.C zeroinitializer, align 2

Minor nit: I think the other test can live in this file, highlighting the 
difference between the 2 cases is nice. Also I think we should add a typedef 
test in here as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

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


[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

2021-08-04 Thread David Goldman via Phabricator via cfe-commits
dgoldman marked an inline comment as done.
dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:535
+/// by range.
+std::vector mergePragmas(std::vector ,
+ std::vector 
,

dgoldman wrote:
> kadircet wrote:
> > dgoldman wrote:
> > > kadircet wrote:
> > > > dgoldman wrote:
> > > > > kadircet wrote:
> > > > > > dgoldman wrote:
> > > > > > > kadircet wrote:
> > > > > > > > dgoldman wrote:
> > > > > > > > > sammccall wrote:
> > > > > > > > > > FWIW the flow control/how we make progress seem hard to 
> > > > > > > > > > follow here to me.
> > > > > > > > > > 
> > > > > > > > > > In particular I think I'm struggling with the statefulness 
> > > > > > > > > > of "is there an open mark group".
> > > > > > > > > > 
> > > > > > > > > > Possible simplifications:
> > > > > > > > > >  - define a dummy root symbol, which seems clearer than the 
> > > > > > > > > > vector + range
> > > > > > > > > >  - avoid reverse-sorting the list of pragma symbols, and 
> > > > > > > > > > just consume from the front of an ArrayRef instead
> > > > > > > > > >  - make the outer loop over pragmas, rather than symbols. 
> > > > > > > > > > It would first check if the pragma belongs directly here or 
> > > > > > > > > > not, and if so, loop over symbols to work out which should 
> > > > > > > > > > become children. This seems very likely to be efficient 
> > > > > > > > > > enough in practice (few pragmas, or most children are 
> > > > > > > > > > grouped into pragmas)
> > > > > > > > > > define a dummy root symbol, which seems clearer than the 
> > > > > > > > > > vector + range
> > > > > > > > > 
> > > > > > > > > I guess? Then we'd take in a `DocumentSymbol & and a 
> > > > > > > > > ArrayRef & (or just by value and then 
> > > > > > > > > return it as well). The rest would be the same though
> > > > > > > > > 
> > > > > > > > > > In particular I think I'm struggling with the statefulness 
> > > > > > > > > > of "is there an open mark group".
> > > > > > > > > 
> > > > > > > > > We need to track the current open group if there is one in 
> > > > > > > > > order to move children to it.
> > > > > > > > > 
> > > > > > > > > > make the outer loop over pragmas, rather than symbols. It 
> > > > > > > > > > would first check if the pragma belongs directly here or 
> > > > > > > > > > not, and if so, loop over symbols to work out which should 
> > > > > > > > > > become children. This seems very likely to be efficient 
> > > > > > > > > > enough in practice (few pragmas, or most children are 
> > > > > > > > > > grouped into pragmas)
> > > > > > > > > 
> > > > > > > > > The important thing here is knowing where the pragma mark 
> > > > > > > > > ends - if it doesn't, it actually gets all of the children. 
> > > > > > > > > So we'd have to peak at the next pragma mark, add all symbols 
> > > > > > > > > before it to us as children, and then potentially recurse to 
> > > > > > > > > nest it inside of a symbol. I'll try it out and see if it's 
> > > > > > > > > simpler.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > ```
> > > > > > > > while(Pragmas) {
> > > > > > > > // We'll figure out where the Pragmas.front() should go.
> > > > > > > > Pragma P = Pragmas.front();
> > > > > > > > DocumentSymbol *Cur = Root;
> > > > > > > > while(Cur->contains(P)) {
> > > > > > > >   auto *OldCur = Cur;
> > > > > > > >   for(auto *C : Cur->children) {
> > > > > > > >  // We assume at most 1 child can contain the pragma (as 
> > > > > > > > pragmas are on a single line, and children have disjoint ranges)
> > > > > > > >  if (C->contains(P)) {
> > > > > > > >  Cur = C;
> > > > > > > >  break;
> > > > > > > >  }
> > > > > > > >   }
> > > > > > > >   // Cur is immediate parent of P
> > > > > > > >   if (OldCur == Cur) {
> > > > > > > > // Just insert P into children if it is not a group and we 
> > > > > > > > are done.
> > > > > > > > // Otherwise we need to figure out when current pragma is 
> > > > > > > > terminated:
> > > > > > > > // if next pragma is not contained in Cur, or is contained in 
> > > > > > > > one of the children, It is at the end of Cur, nest all the 
> > > > > > > > children that appear after P under the symbol node for P.
> > > > > > > > // Otherwise nest all the children that appear after P but 
> > > > > > > > before next pragma under the symbol node for P.
> > > > > > > > // Pop Pragmas and break
> > > > > > > >   }
> > > > > > > > }
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > Does that make sense, i hope i am not missing something 
> > > > > > > > obvious? Complexity-wise in the worst case we'll go all the way 
> > > > > > > > down to a leaf once per pragma, since there will only be a 
> > > > > > > > handful of pragmas most of the time it shouldn't be too bad.
> > > > > > > I've implemented your suggestion. I don't think it's simpler, but 
> > > > > > > LMK, 

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

2021-08-04 Thread David Goldman via Phabricator via cfe-commits
dgoldman marked 2 inline comments as done.
dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:664
+  }
+  // NextPragma is after us but before the next symbol. Our reign is over.
+  break;

kadircet wrote:
> I suppose this is not correct in cases like.
> 
> ```
> #pragma mark - Outer
> foo {
>   #pragma mark - Foo
> }
> bar {
>   # pragma mark - Bar
> }
> baz {
>   # pragma mark - Baz
> }
> ```
> 
> We'll first move `foo` under `Outer`, then start processing `bar` we notice 
> that It comes after pragma `Foo`, and then also notice that pragma `Foo` is 
> not contained in `bar` hence break. but all of `foo,bar,baz` should've been 
> moved to be under `Outer` instead.
Yeah that's a good point, we'd have to do something like your suggested 
solution or maintain more state to consider the children that were moved.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:535
+/// by range.
+std::vector mergePragmas(std::vector ,
+ std::vector 
,

kadircet wrote:
> dgoldman wrote:
> > kadircet wrote:
> > > dgoldman wrote:
> > > > kadircet wrote:
> > > > > dgoldman wrote:
> > > > > > kadircet wrote:
> > > > > > > dgoldman wrote:
> > > > > > > > sammccall wrote:
> > > > > > > > > FWIW the flow control/how we make progress seem hard to 
> > > > > > > > > follow here to me.
> > > > > > > > > 
> > > > > > > > > In particular I think I'm struggling with the statefulness of 
> > > > > > > > > "is there an open mark group".
> > > > > > > > > 
> > > > > > > > > Possible simplifications:
> > > > > > > > >  - define a dummy root symbol, which seems clearer than the 
> > > > > > > > > vector + range
> > > > > > > > >  - avoid reverse-sorting the list of pragma symbols, and just 
> > > > > > > > > consume from the front of an ArrayRef instead
> > > > > > > > >  - make the outer loop over pragmas, rather than symbols. It 
> > > > > > > > > would first check if the pragma belongs directly here or not, 
> > > > > > > > > and if so, loop over symbols to work out which should become 
> > > > > > > > > children. This seems very likely to be efficient enough in 
> > > > > > > > > practice (few pragmas, or most children are grouped into 
> > > > > > > > > pragmas)
> > > > > > > > > define a dummy root symbol, which seems clearer than the 
> > > > > > > > > vector + range
> > > > > > > > 
> > > > > > > > I guess? Then we'd take in a `DocumentSymbol & and a 
> > > > > > > > ArrayRef & (or just by value and then return 
> > > > > > > > it as well). The rest would be the same though
> > > > > > > > 
> > > > > > > > > In particular I think I'm struggling with the statefulness of 
> > > > > > > > > "is there an open mark group".
> > > > > > > > 
> > > > > > > > We need to track the current open group if there is one in 
> > > > > > > > order to move children to it.
> > > > > > > > 
> > > > > > > > > make the outer loop over pragmas, rather than symbols. It 
> > > > > > > > > would first check if the pragma belongs directly here or not, 
> > > > > > > > > and if so, loop over symbols to work out which should become 
> > > > > > > > > children. This seems very likely to be efficient enough in 
> > > > > > > > > practice (few pragmas, or most children are grouped into 
> > > > > > > > > pragmas)
> > > > > > > > 
> > > > > > > > The important thing here is knowing where the pragma mark ends 
> > > > > > > > - if it doesn't, it actually gets all of the children. So we'd 
> > > > > > > > have to peak at the next pragma mark, add all symbols before it 
> > > > > > > > to us as children, and then potentially recurse to nest it 
> > > > > > > > inside of a symbol. I'll try it out and see if it's simpler.
> > > > > > > > 
> > > > > > > > 
> > > > > > > ```
> > > > > > > while(Pragmas) {
> > > > > > > // We'll figure out where the Pragmas.front() should go.
> > > > > > > Pragma P = Pragmas.front();
> > > > > > > DocumentSymbol *Cur = Root;
> > > > > > > while(Cur->contains(P)) {
> > > > > > >   auto *OldCur = Cur;
> > > > > > >   for(auto *C : Cur->children) {
> > > > > > >  // We assume at most 1 child can contain the pragma (as 
> > > > > > > pragmas are on a single line, and children have disjoint ranges)
> > > > > > >  if (C->contains(P)) {
> > > > > > >  Cur = C;
> > > > > > >  break;
> > > > > > >  }
> > > > > > >   }
> > > > > > >   // Cur is immediate parent of P
> > > > > > >   if (OldCur == Cur) {
> > > > > > > // Just insert P into children if it is not a group and we 
> > > > > > > are done.
> > > > > > > // Otherwise we need to figure out when current pragma is 
> > > > > > > terminated:
> > > > > > > // if next pragma is not contained in Cur, or is contained in one 
> > > > > > > of the children, It is at the end of Cur, nest all the children 
> > > > > > > that appear after P under the symbol node for P.
> > > > > > > // Otherwise nest all the 

[PATCH] D107241: [AIX] Define __THW_BIG_ENDIAN__ macro

2021-08-04 Thread Chris Bowler via Phabricator via cfe-commits
cebowleratibm accepted this revision.
cebowleratibm 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/D107241/new/

https://reviews.llvm.org/D107241

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


[PATCH] D106909: [clang] Add clang builtins support for gfx90a

2021-08-04 Thread Anshil Gandhi via Phabricator via cfe-commits
gandhi21299 added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsAMDGPU.def:201
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_f32, "ff*1f", "t", 
"gfx90a-insts")
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_v2f16, "V2hV2h*1V2h", "t", 
"gfx90a-insts")
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fmin_f64, "dd*1d", "t", 
"gfx90a-insts")

I tried add target feature gfx908-insts for this builtin but the frontend 
complains that it should have target feature gfx90a-insts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106909

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


[PATCH] D106909: [clang] Add clang builtins support for gfx90a

2021-08-04 Thread Anshil Gandhi via Phabricator via cfe-commits
gandhi21299 updated this revision to Diff 364152.
gandhi21299 added a comment.

updating test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106909

Files:
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx1030.cl
  clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
  clang/test/CodeGenOpenCL/builtins-fp-atomics-unsupported-gfx908.cl

Index: clang/test/CodeGenOpenCL/builtins-fp-atomics-unsupported-gfx908.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/builtins-fp-atomics-unsupported-gfx908.cl
@@ -0,0 +1,8 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -triple amdgcn-amd-amdhsa -target-cpu gfx908 \
+// RUN:   -verify -S -o - %s
+
+void test_global_add(__global double *addr, double x) {
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f64(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fadd_f64' needs target feature gfx90a-insts}}
+}
Index: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -triple amdgcn-amd-amdhsa -target-cpu gfx90a \
+// RUN:   %s -S -emit-llvm -o - | FileCheck %s -check-prefix=CHECK
+
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -triple amdgcn-amd-amdhsa -target-cpu gfx90a \
+// RUN:   -S -o - %s | FileCheck -check-prefix=GFX90A %s
+
+typedef half __attribute__((ext_vector_type(2))) half2;
+
+// CHECK-LABEL: test_global_add
+// CHECK: call double @llvm.amdgcn.global.atomic.fadd.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_global_add$local:
+// GFX90A:  global_atomic_add_f64
+void test_global_add(__global double *addr, double x) {
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f64(addr, x);
+}
+
+// CHECK-LABEL: test_global_add_half2
+// CHECK: call <2 x half> @llvm.amdgcn.global.atomic.fadd.v2f16.p1v2f16.v2f16(<2 x half> addrspace(1)* %{{.*}}, <2 x half> %{{.*}})
+// GFX90A-LABEL:  test_global_add_half2
+// GFX90A:  global_atomic_pk_add_f16 v2, v[0:1], v2, off glc
+void test_global_add_half2(__global half2 *addr, half2 x) {
+  half2 *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fadd_v2f16(addr, x);
+}
+
+// CHECK-LABEL: test_global_global_min
+// CHECK: call double @llvm.amdgcn.global.atomic.fmin.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_global_global_min$local
+// GFX90A:  global_atomic_min_f64
+void test_global_global_min(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fmin_f64(addr, x);
+}
+
+// CHECK-LABEL: test_global_max
+// CHECK: call double @llvm.amdgcn.global.atomic.fmax.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_global_max$local
+// GFX90A:  global_atomic_max_f64
+void test_global_max(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_global_atomic_fmax_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_add_local
+// CHECK: call double @llvm.amdgcn.flat.atomic.fadd.f64.p3f64.f64(double addrspace(3)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_add_local$local
+// GFX90A:  ds_add_rtn_f64
+void test_flat_add_local(__local double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fadd_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_global_add
+// CHECK: call double @llvm.amdgcn.flat.atomic.fadd.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_global_add$local
+// GFX90A:  global_atomic_add_f64
+void test_flat_global_add(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fadd_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_min_constant
+// CHECK: call double @llvm.amdgcn.flat.atomic.fmin.f64.p0f64.f64(double* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_min_constant$local
+// GFX90A:  flat_atomic_min_f64
+void test_flat_min_constant(__generic double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fmin_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_global_min
+// CHECK: call double @llvm.amdgcn.flat.atomic.fmin.f64.p1f64.f64(double addrspace(1)* %{{.*}}, double %{{.*}})
+// GFX90A:  test_flat_global_min$local
+// GFX90A:  global_atomic_min_f64
+void test_flat_global_min(__global double *addr, double x){
+  double *rtn;
+  *rtn = __builtin_amdgcn_flat_atomic_fmin_f64(addr, x);
+}
+
+// CHECK-LABEL: test_flat_max_constant
+// CHECK: call double @llvm.amdgcn.flat.atomic.fmax.f64.p0f64.f64(double* %{{.*}}, double %{{.*}})
+// GFX90A-LABEL:  test_flat_max_constant$local
+// GFX90A:  flat_atomic_max_f64
+void test_flat_max_constant(__generic double *addr, double x){
+  

[PATCH] D107477: [LLVM][AST][NFC] Resolve Fixme: Make CXXRecordDecl *Record const.

2021-08-04 Thread Alf via Phabricator via cfe-commits
gAlfonso-bit created this revision.
gAlfonso-bit added a project: LLVM.
gAlfonso-bit requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107477

Files:
  clang/include/clang/AST/ComparisonCategories.h


Index: clang/include/clang/AST/ComparisonCategories.h
===
--- clang/include/clang/AST/ComparisonCategories.h
+++ clang/include/clang/AST/ComparisonCategories.h
@@ -115,8 +115,7 @@
 public:
   /// The declaration for the comparison category type from the
   /// standard library.
-  // FIXME: Make this const
-  CXXRecordDecl *Record = nullptr;
+  const CXXRecordDecl *Record = nullptr;
 
   /// The Kind of the comparison category type
   ComparisonCategoryType Kind;


Index: clang/include/clang/AST/ComparisonCategories.h
===
--- clang/include/clang/AST/ComparisonCategories.h
+++ clang/include/clang/AST/ComparisonCategories.h
@@ -115,8 +115,7 @@
 public:
   /// The declaration for the comparison category type from the
   /// standard library.
-  // FIXME: Make this const
-  CXXRecordDecl *Record = nullptr;
+  const CXXRecordDecl *Record = nullptr;
 
   /// The Kind of the comparison category type
   ComparisonCategoryType Kind;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D94639: [DebugInfo][CodeView] Change in line tables only mode to emit parent/context scopes for functions, using declarations for types

2021-08-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In D94639#2918559 , @SeTSeR wrote:

> Thank you for the detailed answer. I'll discuss our use case with our team. 
> Should I create a separate ticket for this? Or maybe it would be better if I 
> submitted the PR adding this flag?

It's free to upload patches, and they help drive discussion forward, but I 
think the main thing is to be really clear about what problem this solves and 
what the costs are. An easy metric for the cost would be the binary size of 
clang with `-g1` and how much it increases in the new mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94639

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


[PATCH] D107420: [sema] Disallow __builtin_mul_overflow under special condition.

2021-08-04 Thread Freddy, Ye via Phabricator via cfe-commits
FreddyYe marked an inline comment as done.
FreddyYe added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8351-8353
+def err_overflow_builtin_special_combination_max_size : Error<
+  "__builtin_mul_overflow does not suport unsigned overflow check after 
convention "
+  "more than %0 bits">;

The new diagnostic is here. Forgot to update tests since I've not rebuilt 
completely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107420

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


[PATCH] D107420: [sema] Disallow __builtin_mul_overflow under special condition.

2021-08-04 Thread Freddy, Ye via Phabricator via cfe-commits
FreddyYe marked an inline comment as done.
FreddyYe added a comment.

Addressed. THX review!




Comment at: clang/test/Sema/builtins-overflow.c:45
+unsigned _ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, ); // expected-error 
{{__builtin_mul_overflow does not support special combination operands (signed, 
signed, unsigned*) of more than 64 bits}}
+  }

aaron.ballman wrote:
> Yeah, this diagnostic really doesn't tell me what's going wrong with the code 
> or how to fix it. Do we basically want to prevent using larger-than-64-bit 
> argument types with mixed signs? Or are there other problematic circumstances?
Yes, let me try to refine. I can explain more what happened to such input 
combination.
According to gcc's the definition on this builtin: 
https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html
'These built-in functions promote the first two operands into infinite 
precision signed type and perform multiply on those promoted operands. The 
result is then cast to the type the third pointer argument points to and stored 
there.' 

Since signing integer has a smaller range than unsigned integer. And now the 
API in compiler-rt (`__muloti4`) to checking 128 integer's multiplying is 
implemented in signed version. So the overflow max absolute value it can check 
is 2^127. When the result input is larger equal than 128 bits, `__muloti4` has 
no usage. We should prevent this situation for now. Or the backend will crush 
as the example shows.

I found the input operand doesn't need both of them larger than 64 bits, but 
just the sum of their larger 128. I'll refine in my patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107420

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


  1   2   >