[PATCH] D14982: ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply Add/Subtract.

[pengbin via Phabricator via cfe-commits]

pengbins added inline comments.
Herald added a subscriber: kristof.beyls.



Comment at: cfe/trunk/include/clang/Basic/arm_neon.td:377
+def OP_QRDMLAH : Op<(call "vqadd", $p0, (call "vqrdmulh", $p1, $p2))>;
+def OP_QRDMLSH : Op<(call "vqsub", $p0, (call "vqrdmulh", $p1, $p2))>;
+def OP_QRDMLAH_LN : Op<(call "vqadd", $p0, (call "vqrdmulh", $p1, (splat $p2, 
$p3)))>;

@labrinea 
It seems QRDMLSH(p0, p1, p2) is not equivelent to with  vqsub( p0, vqrdmulh(p1, 
p2)).

  - QRDMLSH(p0, p1, p2)

```
accum = ((p0 << esize) - 2 * (p1 * p2) + rounding_const);
ret = SignedSatQ(accum >> esize, esize);
```
//p0<< esize + rounding_const //

  - vqsub( p0, vqrdmulh(p1, p2))

```
temp = SignedSatQ( (2 * (p1 * p2) + rounding_const) >> esize );
ret = SignedSat( p0  -  temp);
```
//p0<< esize - rounding_const //


Here is an example where the results are not same.
vqrdmlshq_s16 ( -197,   -512, 11040)   = -24
vqsubq_s16( -197, vqrdmulhq_n_s16(-512, 11040)) = -25


Repository:
  rL LLVM

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

https://reviews.llvm.org/D14982

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

[PATCH] D97375: [clang][cli] Add MarshallingInfoEnum multiclass

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG88e45f00c156: [clang][cli] Add MarshallingInfoEnum 
multiclass (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97375

Files:
  clang/docs/InternalsManual.rst
  clang/include/clang/Driver/Options.td
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -211,6 +211,14 @@
   code Denormalizer = "makeBooleanOptionDenormalizer("#value#")";
 }
 
+// Marshalling info for enums. Typically used with `Values`, `NormalizedValues`
+// and `NormalizedValuesScope` mixins.
+class MarshallingInfoEnum
+  : MarshallingInfo {
+  code Normalizer = "normalizeSimpleEnum";
+  code Denormalizer = "denormalizeSimpleEnum";
+}
+
 // Mixins for additional marshalling attributes.
 
 class ShouldParseIf { code ShouldParse = condition; }
@@ -219,10 +227,6 @@
 class Denormalizer { code Denormalizer = denormalizer; }
 class NormalizedValuesScope { code NormalizedValuesScope = scope; }
 class NormalizedValues definitions> { list NormalizedValues = definitions; } 
-class AutoNormalizeEnum {
-  code Normalizer = "normalizeSimpleEnum";
-  code Denormalizer = "denormalizeSimpleEnum";
-}
 class ValueMerger { code ValueMerger = merger; }
 class ValueExtractor { code ValueExtractor = extractor; }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -427,11 +427,10 @@
 
 // FIXME: Diagnose if target does not support protected visibility.
 class MarshallingInfoVisibility
-  : MarshallingInfoString,
+  : MarshallingInfoEnum,
 Values<"default,hidden,internal,protected">,
 NormalizedValues<["DefaultVisibility", "HiddenVisibility",
-  "HiddenVisibility", "ProtectedVisibility"]>,
-AutoNormalizeEnum {}
+  "HiddenVisibility", "ProtectedVisibility"]> {}
 
 // Key paths that are constant during parsing of options with the same key path prefix.
 defvar cplusplus = LangOpts<"CPlusPlus">;
@@ -1062,7 +1061,7 @@
 HelpText<"Embed LLVM bitcode (option: off, all, bitcode, marker)">,
 Values<"off,all,bitcode,marker">, NormalizedValuesScope<"CodeGenOptions">,
 NormalizedValues<["Embed_Off", "Embed_All", "Embed_Bitcode", "Embed_Marker"]>,
-MarshallingInfoString, "Embed_Off">, AutoNormalizeEnum;
+MarshallingInfoEnum, "Embed_Off">;
 def fembed_bitcode : Flag<["-"], "fembed-bitcode">, Group,
   Alias, AliasArgs<["all"]>,
   HelpText<"Embed LLVM IR bitcode as data">;
@@ -1248,7 +1247,7 @@
 Flags<[CC1Option]>, Values<"unspecified,standalone,objc,swift,swift-5.0,swift-4.2,swift-4.1">,
 NormalizedValuesScope<"LangOptions::CoreFoundationABI">,
 NormalizedValues<["ObjectiveC", "ObjectiveC", "ObjectiveC", "Swift5_0", "Swift5_0", "Swift4_2", "Swift4_1"]>,
-MarshallingInfoString, "ObjectiveC">, AutoNormalizeEnum;
+MarshallingInfoEnum, "ObjectiveC">;
 defm constant_cfstrings : BoolFOption<"constant-cfstrings",
   LangOpts<"NoConstantCFStrings">, DefaultFalse,
   NegFlag,
@@ -1356,8 +1355,7 @@
   Values<"dwarf,sjlj,seh,wasm">,
   NormalizedValuesScope<"llvm::ExceptionHandling">,
   NormalizedValues<["DwarfCFI", "SjLj", "WinEH", "Wasm"]>,
-  MarshallingInfoString, "None">,
-  AutoNormalizeEnum;
+  MarshallingInfoEnum, "None">;
 def exception_model_EQ : Joined<["-"], "exception-model=">,
   Flags<[CC1Option, NoDriverOption]>, Alias;
 def fignore_exceptions : Flag<["-"], "fignore-exceptions">, Group, Flags<[CC1Option]>,
@@ -1380,7 +1378,7 @@
   HelpText<"Specifies the exception behavior of floating-point operations.">,
   Values<"ignore,maytrap,strict">, NormalizedValuesScope<"LangOptions">,
   NormalizedValues<["FPE_Ignore", "FPE_MayTrap", "FPE_Strict"]>,
-  MarshallingInfoString, "FPE_Ignore">, AutoNormalizeEnum;
+  MarshallingInfoEnum, "FPE_Ignore">;
 defm fast_math : OptInFFlag<"fast-math", "Allow aggressive, lossy floating-point optimizations", "", "", [],
   LangOpts<"FastMath">, [cl_fast_relaxed_math.KeyPath]>;
 def menable_unsafe_fp_math : Flag<["-"], "menable-unsafe-fp-math">, Flags<[CC1Option]>,
@@ -1820,10 +1818,9 @@
   NormalizedValues<["LangOptions::LaxVectorConversionKind::None",
 "LangOptions::LaxVectorConversionKind::Integer",
 "LangOptions::LaxVectorConversionKind::All"]>,
-  MarshallingInfoString,
-!strconcat(open_cl.KeyPath, " ? LangOptions::LaxVectorConversionKind::None"
-" : LangOptions::LaxVectorConversionKind::All")>,
-  AutoNormalizeEnum;
+  MarshallingInfoEnum,
+  !strconcat(open_cl.KeyPath, " ? 

[clang] 88e45f0 - [clang][cli] Add MarshallingInfoEnum multiclass

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-02-25T08:47:18+01:00
New Revision: 88e45f00c156170ed562bbacad3b2a21633c0f7a

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

LOG: [clang][cli] Add MarshallingInfoEnum multiclass

This patch introduces a tablegen multiclass called `MarshallingInfoEnum`. It 
has the same semantics as `MarshallingInfoString` had in combination with 
`AutoNormalizeEnum`, but it's easier to use and follows the convention used for 
other `MarshallingInfoXxx` multiclasses.

Reviewed By: dexonsmith

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

Added: 


Modified: 
clang/docs/InternalsManual.rst
clang/include/clang/Driver/Options.td
llvm/include/llvm/Option/OptParser.td

Removed: 




diff  --git a/clang/docs/InternalsManual.rst b/clang/docs/InternalsManual.rst
index da17d72906d7..ec018755f491 100644
--- a/clang/docs/InternalsManual.rst
+++ b/clang/docs/InternalsManual.rst
@@ -940,23 +940,22 @@ and the result is assigned to the key path on success.
 
 **Enumeration**
 
-The key path defaults to the value specified in ``MarshallingInfoString``
-prefixed by the contents of ``NormalizedValuesScope`` and ``::``. This ensures
-correct reference to an enum case is formed even if the enum resides in 
diff erent
+The key path defaults to the value specified in ``MarshallingInfoEnum`` 
prefixed
+by the contents of ``NormalizedValuesScope`` and ``::``. This ensures correct
+reference to an enum case is formed even if the enum resides in 
diff erent
 namespace or is an enum class. If the value present on command line does not
 match any of the comma-separated values from ``Values``, an error diagnostics 
is
 issued. Otherwise, the corresponding element from ``NormalizedValues`` at the
 same index is assigned to the key path (also correctly scoped). The number of
 comma-separated string values and elements of the array within
-``NormalizedValues`` must match. The ``AutoNormalizeEnum`` mix-in denotes the
-key path option should be treated as an enum and not as a string.
+``NormalizedValues`` must match.
 
 .. code-block::
 
   def mthread_model : Separate<["-"], "mthread-model">, Flags<[CC1Option]>,
 Values<"posix,single">, NormalizedValues<["POSIX", "Single"]>,
 NormalizedValuesScope<"LangOptions::ThreadModelKind">,
-MarshallingInfoString, "POSIX">, AutoNormalizeEnum;
+MarshallingInfoEnum, "POSIX">;
 
 ..
   Intentionally omitting MarshallingInfoBitfieldFlag. It's adding some

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index e4a6ef9cb2f1..57def483e6de 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -427,11 +427,10 @@ multiclass BoolGOption
-  : MarshallingInfoString,
+  : MarshallingInfoEnum,
 Values<"default,hidden,internal,protected">,
 NormalizedValues<["DefaultVisibility", "HiddenVisibility",
-  "HiddenVisibility", "ProtectedVisibility"]>,
-AutoNormalizeEnum {}
+  "HiddenVisibility", "ProtectedVisibility"]> {}
 
 // Key paths that are constant during parsing of options with the same key 
path prefix.
 defvar cplusplus = LangOpts<"CPlusPlus">;
@@ -1062,7 +1061,7 @@ def fembed_bitcode_EQ : Joined<["-"], "fembed-bitcode=">,
 HelpText<"Embed LLVM bitcode (option: off, all, bitcode, marker)">,
 Values<"off,all,bitcode,marker">, NormalizedValuesScope<"CodeGenOptions">,
 NormalizedValues<["Embed_Off", "Embed_All", "Embed_Bitcode", 
"Embed_Marker"]>,
-MarshallingInfoString, "Embed_Off">, 
AutoNormalizeEnum;
+MarshallingInfoEnum, "Embed_Off">;
 def fembed_bitcode : Flag<["-"], "fembed-bitcode">, Group,
   Alias, AliasArgs<["all"]>,
   HelpText<"Embed LLVM IR bitcode as data">;
@@ -1248,7 +1247,7 @@ def fcf_runtime_abi_EQ : Joined<["-"], 
"fcf-runtime-abi=">, Group,
 Flags<[CC1Option]>, 
Values<"unspecified,standalone,objc,swift,swift-5.0,swift-4.2,swift-4.1">,
 NormalizedValuesScope<"LangOptions::CoreFoundationABI">,
 NormalizedValues<["ObjectiveC", "ObjectiveC", "ObjectiveC", "Swift5_0", 
"Swift5_0", "Swift4_2", "Swift4_1"]>,
-MarshallingInfoString, "ObjectiveC">, 
AutoNormalizeEnum;
+MarshallingInfoEnum, "ObjectiveC">;
 defm constant_cfstrings : BoolFOption<"constant-cfstrings",
   LangOpts<"NoConstantCFStrings">, DefaultFalse,
   NegFlag,
@@ -1356,8 +1355,7 @@ def exception_model : Separate<["-"], "exception-model">,
   Values<"dwarf,sjlj,seh,wasm">,
   NormalizedValuesScope<"llvm::ExceptionHandling">,
   NormalizedValues<["DwarfCFI", "SjLj", "WinEH", "Wasm"]>,
-  MarshallingInfoString, "None">,
-  AutoNormalizeEnum;
+  MarshallingInfoEnum, "None">;
 def exception_model_EQ : Joined<["-"], "exception-model=">,
   Flags<[CC1Option, NoDriverOption]>, Alias;
 def 

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

[Yuanfang Chen via Phabricator via cfe-commits]

ychen created this revision.
ychen added reviewers: MaskRay, rnk, tejohnson, qcolombet, anemet.
Herald added subscribers: dexonsmith, kerbowa, hiraditya, nhaehnle, jvesely.
ychen requested review of this revision.
Herald added projects: clang, LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits.

The situation with inline asm/MC error reporting is kind of messy at the
moment. The errors from MC layout are not reliably propagated(D96931 
) and users
have to specify an inlineasm handler separately to get inlineasm
diagnose. The latter issue is not a correctness issue but could be improved.

- Kill LLVMContext inlineasm diagnose handler and migrate it to use

DiagnoseInfo/DiagnoseHandler.

- Introduce `DiagnoseInfoSrcMgr` to diagnose SourceMgr backed errors.

This covers use cases like inline asm, MC and any clients using
SourceMgr.

- Move AsmPrinter::SrcMgrDiagInfo and its instance to MCContext. The

next step is to combine MCContext::SrcMgr and MCContext::InlineSrcMgr
because in all use cases, only one of them is used.

- If LLVMContext is available, let MCContext uses LLVMContext's diagnose

handler; if LLVMContext is not available, MCContext uses its own default
diagnose handler which just print SMDiagnostic.

- Change a few clients(Clang, llc, lldb) to use new way of reporting.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97449

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/MC/MCContext.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
  llvm/lib/CodeGen/MachineModuleInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/test/CodeGen/AMDGPU/lds-initializer.ll
  llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
  llvm/test/CodeGen/XCore/section-name.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -290,6 +290,22 @@
   bool *HasError;
   LLCDiagnosticHandler(bool *HasErrorPtr) : HasError(HasErrorPtr) {}
   bool handleDiagnostics(const DiagnosticInfo ) override {
+if (DI.getKind() == llvm::DK_SrcMgr) {
+  const auto  = cast(DI);
+  const SMDiagnostic  = DISM.getSMDiag();
+
+  if (SMD.getKind() == SourceMgr::DK_Error)
+*HasError = true;
+
+  SMD.print(nullptr, errs());
+
+  // For testing purposes, we print the LocCookie here.
+  if (DISM.isInlineAsmDiag() && DISM.getLocCookie())
+WithColor::note() << "!srcloc = " << DISM.getLocCookie() << "\n";
+
+  return true;
+}
+
 if (DI.getSeverity() == DS_Error)
   *HasError = true;
 
@@ -305,19 +321,6 @@
   }
 };
 
-static void InlineAsmDiagHandler(const SMDiagnostic , void *Context,
- unsigned LocCookie) {
-  bool *HasError = static_cast(Context);
-  if (SMD.getKind() == SourceMgr::DK_Error)
-*HasError = true;
-
-  SMD.print(nullptr, errs());
-
-  // For testing purposes, we print the LocCookie here.
-  if (LocCookie)
-WithColor::note() << "!srcloc = " << LocCookie << "\n";
-}
-
 // main - Entry point for the llc compiler.
 //
 int main(int argc, char **argv) {
@@ -367,7 +370,6 @@
   bool HasError = false;
   Context.setDiagnosticHandler(
   std::make_unique());
-  Context.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, );
 
   Expected> RemarksFileOrErr =
   setupLLVMOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
Index: llvm/test/CodeGen/XCore/section-name.ll
===
--- llvm/test/CodeGen/XCore/section-name.ll
+++ llvm/test/CodeGen/XCore/section-name.ll
@@ -1,4 +1,4 @@
-; RUN: llc < %s -march=xcore -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc < %s -march=xcore -o /dev/null 2>&1 | FileCheck %s
 
 @bar = internal global i32 zeroinitializer
 
Index: llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
===
--- llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
+++ llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll
@@ -1,5 +1,5 @@
-; RUN: llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
-; RUN: llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
 
 ; CHECK: lds: unsupported initializer for address space
 
Index: llvm/test/CodeGen/AMDGPU/lds-initializer.ll
===
--- llvm/test/CodeGen/AMDGPU/lds-initializer.ll
+++ llvm/test/CodeGen/AMDGPU/lds-initializer.ll
@@ -1,5 +1,5 @@
-; RUN: llc 

[PATCH] D96838: Add GNU attribute 'retain'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay abandoned this revision.
MaskRay added a comment.

Thanks folks for review. Folks are more happy with approach 1 on 
https://lists.llvm.org/pipermail/llvm-dev/2021-February/148760.html , so I am 
abandoning this.

I have copied the documentation and tests to D97447 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D97447: Add GNU attribute 'retain'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: aaron.ballman, rjmccall, rnk, rsmith.
Herald added a subscriber: pengfei.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For ELF targets, GCC 11 will set SHF_GNU_RETAIN on the section of a
`__attribute__((retain))` function/variable to prevent linker garbage
collection. (See AttrDocs.td for the linker support).

This patch adds `retain` functions/variables to the `llvm.used` list, which has
the desired linker GC semantics. Note: `retain` does not imply `used`,
so an unused function/variable can be dropped by Sema.

Before 'retain' was introduced, previous ELF solutions require inline asm or
linker tricks, e.g.  `asm volatile(".reloc 0, R_X86_64_NONE, target");`
(architecture dependent) or define a non-local symbol in the section and use
`ld -u`. There was no elegant source-level solution.

As of this patch, llvm.used and llvm.compiler.used have the same
behaviors for ELF. `__attribute__((retain))` does not set `SHF_GNU_RETAIN` yet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97447

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/attr-retain.c
  clang/test/CodeGenCXX/attr-retain.cpp
  clang/test/Sema/attr-retain.c

Index: clang/test/Sema/attr-retain.c
===
--- /dev/null
+++ clang/test/Sema/attr-retain.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wunused-function
+
+/// We allow 'retain' on non-ELF targets because 'retain' is often used together
+/// with 'used'. 'used' has GC root semantics on macOS and Windows. We want
+/// users to just write retain,used and don't need to dispatch on binary formats.
+
+extern char test1[] __attribute__((retain));   // expected-warning {{'retain' attribute ignored on a non-definition declaration}}
+extern const char test2[] __attribute__((retain)); // expected-warning {{'retain' attribute ignored on a non-definition declaration}}
+const char test3[] __attribute__((retain)) = "";
+
+struct __attribute__((retain)) s { // expected-warning {{'retain' attribute only applies to variables with non-local storage, functions, and Objective-C methods}}
+};
+
+void foo() {
+  static int a __attribute__((retain));
+  int b __attribute__((retain)); // expected-warning {{'retain' attribute only applies to variables with non-local storage, functions, and Objective-C methods}}
+  (void)a;
+  (void)b;
+}
+
+__attribute__((retain,used)) static void f0() {}
+__attribute__((retain)) static void f1() {} // expected-warning {{unused function 'f1'}}
+__attribute__((retain)) void f2() {}
+
+/// Test attribute merging.
+int tentative;
+int tentative __attribute__((retain));
+extern int tentative;
+int tentative = 0;
Index: clang/test/CodeGenCXX/attr-retain.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/attr-retain.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -Werror %s -o - | FileCheck %s
+
+// CHECK:  @llvm.used = appending global [7 x i8*]
+// CHECK-SAME:   @_ZN2X0C2Ev
+// CHECK-SAME:   @_ZN2X0C1Ev
+// CHECK-SAME:   @_ZN2X0D2Ev
+// CHECK-SAME:   @_ZN2X0D1Ev
+// CHECK-SAME:   @_ZN2X16Nested2f1Ev
+// CHECK-SAME:   @_ZN10merge_declL4funcEv
+// CHECK-SAME:   @_ZN18instantiate_member1SIiE1fEv
+
+struct X0 {
+  // CHECK: define linkonce_odr{{.*}} @_ZN2X0C1Ev({{.*}}
+  __attribute__((used, retain)) X0() {}
+  // CHECK: define linkonce_odr{{.*}} @_ZN2X0D1Ev({{.*}}
+  __attribute__((used, retain)) ~X0() {}
+};
+
+struct X1 {
+  struct Nested {
+// CHECK-NOT: 2f0Ev
+// CHECK: define linkonce_odr{{.*}} @_ZN2X16Nested2f1Ev({{.*}}
+void __attribute__((retain)) f0() {}
+void __attribute__((used, retain)) f1() {}
+  };
+};
+
+// CHECK: define internal void @_ZN10merge_declL4funcEv(){{.*}}
+namespace merge_decl {
+static void func();
+void bar() { void func() __attribute__((used, retain)); }
+static void func() {}
+} // namespace merge_decl
+
+namespace instantiate_member {
+template 
+struct S {
+  void __attribute__((used, retain)) f() {}
+};
+
+void test() {
+  // CHECK: define linkonce_odr{{.*}} void @_ZN18instantiate_member1SIiE1fEv({{.*}}
+  S a;
+}
+} // namespace instantiate_member
Index: clang/test/CodeGen/attr-retain.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-retain.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+/// Set !retain regardless of the target. The backend will lower !retain to
+/// SHF_GNU_RETAIN on ELF and ignore the metadata for other binary formats.
+// CHECK:  @c0 ={{.*}} constant i32 {{.*}}
+// CHECK:  @foo.l0 = internal global i32 {{.*}}
+// CHECK:  @g0 ={{.*}} global i32 {{.*}}

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: aaron.ballman, rjmccall, rnk, rsmith.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

An global value in the `llvm.used` list does not have GC root semantics on ELF 
targets.
This will be changed in a subsequent backend patch.

Change some `llvm.used` in the ELF code path to use `llvm.compiler.used` to
prevent undesired GC root semantics.

GNU ld has a rule "__start_/__stop_ references from a live input section retain 
the associated C identifier name sections",
which LLD may change in the future (D96914 ).
For `llvm.used` global values defined in a C identifier name section, keep 
using `llvm.used` so that
the future LLD change will not affect them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97446

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/2005-12-04-AttributeUsed.c
  clang/test/CodeGen/attr-msp430.c
  clang/test/CodeGen/attr-target-mv.c
  clang/test/CodeGen/attr-used.c
  clang/test/CodeGen/attr-x86-interrupt.c
  clang/test/CodeGen/keep-static-consts.cpp
  clang/test/CodeGenCUDA/llvm-used.cu
  clang/test/CodeGenCXX/attr-x86-interrupt.cpp
  clang/test/CodeGenCXX/extern-c.cpp

Index: clang/test/CodeGenCXX/extern-c.cpp
===
--- clang/test/CodeGenCXX/extern-c.cpp
+++ clang/test/CodeGenCXX/extern-c.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -o - | FileCheck %s
 namespace foo {
 
 // CHECK-NOT: @a = global
@@ -70,7 +70,7 @@
 __attribute__((used)) static int duplicate_internal_fn() { return 0; }
   }
 
-  // CHECK: @llvm.used = appending global {{.*}} @internal_var {{.*}} @internal_fn 
+  // CHECK: @llvm.compiler.used = appending global {{.*}} @internal_var {{.*}} @internal_fn
 
   // CHECK-NOT: @unused
   // CHECK-NOT: @duplicate_internal
Index: clang/test/CodeGenCXX/attr-x86-interrupt.cpp
===
--- clang/test/CodeGenCXX/attr-x86-interrupt.cpp
+++ clang/test/CodeGenCXX/attr-x86-interrupt.cpp
@@ -17,11 +17,11 @@
 struct St {
 static void foo9(int *a) __attribute__((interrupt)) {}
 };
-// X86_64_LINUX: @llvm.used = appending global [3 x i8*] [i8* bitcast (void (i32*, i64)* @{{.*}}foo7{{.*}} to i8*), i8* bitcast (void (i32*)* @{{.*}}foo8{{.*}} to i8*), i8* bitcast (void (i32*)* @{{.*}}foo9{{.*}} to i8*)], section "llvm.metadata"
+// X86_64_LINUX: @llvm.compiler.used = appending global [3 x i8*] [i8* bitcast (void (i32*, i64)* @{{.*}}foo7{{.*}} to i8*), i8* bitcast (void (i32*)* @{{.*}}foo8{{.*}} to i8*), i8* bitcast (void (i32*)* @{{.*}}foo9{{.*}} to i8*)], section "llvm.metadata"
 // X86_64_LINUX: define{{.*}} x86_intrcc void @{{.*}}foo7{{.*}}(i32* byval(i32) %{{.+}}, i64 %{{.+}})
 // X86_64_LINUX: define{{.*}} x86_intrcc void @{{.*}}foo8{{.*}}(i32* byval(i32) %{{.+}})
 // X86_64_LINUX: define linkonce_odr x86_intrcc void @{{.*}}foo9{{.*}}(i32* byval(i32) %{{.+}})
-// X86_LINUX: @llvm.used = appending global [3 x i8*] [i8* bitcast (void (i32*, i32)* @{{.*}}foo7{{.*}} to i8*), i8* bitcast (void (i32*)* @{{.*}}foo8{{.*}} to i8*), i8* bitcast (void (i32*)* @{{.*}}foo9{{.*}} to i8*)], section "llvm.metadata"
+// X86_LINUX: @llvm.compiler.used = appending global [3 x i8*] [i8* bitcast (void (i32*, i32)* @{{.*}}foo7{{.*}} to i8*), i8* bitcast (void (i32*)* @{{.*}}foo8{{.*}} to i8*), i8* bitcast (void (i32*)* @{{.*}}foo9{{.*}} to i8*)], section "llvm.metadata"
 // X86_LINUX: define{{.*}} x86_intrcc void @{{.*}}foo7{{.*}}(i32* byval(i32) %{{.+}}, i32 %{{.+}})
 // X86_LINUX: define{{.*}} x86_intrcc void @{{.*}}foo8{{.*}}(i32* byval(i32) %{{.+}})
 // X86_LINUX: define linkonce_odr x86_intrcc void @{{.*}}foo9{{.*}}(i32* byval(i32) %{{.+}})
Index: clang/test/CodeGenCUDA/llvm-used.cu
===
--- clang/test/CodeGenCUDA/llvm-used.cu
+++ clang/test/CodeGenCUDA/llvm-used.cu
@@ -4,5 +4,5 @@
 // Make sure we emit the proper addrspacecast for llvm.used.  PR22383 exposed an
 // issue where we were generating a bitcast instead of an addrspacecast.
 
-// CHECK: @llvm.used = appending global [1 x i8*] [i8* addrspacecast (i8 addrspace(1)* bitcast ([0 x i32] addrspace(1)* @a to i8 addrspace(1)*) to i8*)], section "llvm.metadata"
+// CHECK: @llvm.compiler.used = appending global [1 x i8*] [i8* addrspacecast (i8 addrspace(1)* bitcast ([0 x i32] addrspace(1)* @a to i8 addrspace(1)*) to i8*)], section "llvm.metadata"
 __attribute__((device)) __attribute__((__used__)) int a[] = {};
Index: clang/test/CodeGen/keep-static-consts.cpp
===
--- clang/test/CodeGen/keep-static-consts.cpp
+++ 

[PATCH] D97445: [clang][sema] Ignore xor-used-as-pow if both sides are macros

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: xbolva00, aaron.ballman.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I've read both https://reviews.llvm.org/D63423 and 
https://reviews.llvm.org/D66397, but I think both of them miss the case where 
both RHS and LHS of the XOR are macros, and thus the warning should not be 
emitted (I think).

I ran into this while compiling elfutils with clang and all the available 
changes to the code seem non-sensical and the warning in clang seems misplaced 
given that both operands are macros, even if the left side evaluates to 2 and 
the right side to 10, for example. They are just used as state.

I know this warning (in conjunction with macros especially) has been discussed 
a lot in the past (as far as I know regarding the Chrome code base), but this 
patch is very conservative regarding the change to the warning semantics. And 
the case it changes was not even covered by the tests. I've added a test for it 
now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97445

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-xor-as-pow.cpp


Index: clang/test/SemaCXX/warn-xor-as-pow.cpp
===
--- clang/test/SemaCXX/warn-xor-as-pow.cpp
+++ clang/test/SemaCXX/warn-xor-as-pow.cpp
@@ -65,6 +65,7 @@
 
   res = 2 ^ 0x4;
   res = 2 ^ 04;
+  res = TWO ^ TEN;
   res = 0x2 ^ 10;
   res = 0X2 ^ 10;
   res = 02 ^ 10;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -12102,6 +12102,11 @@
   if (Loc.isMacroID())
 return;
 
+  // Do not diagnose if both LHS and RHS are macros
+  if (XorLHS.get()->getExprLoc().isMacroID() &&
+  XorRHS.get()->getExprLoc().isMacroID())
+return;
+
   bool Negative = false;
   bool ExplicitPlus = false;
   const auto *LHSInt = dyn_cast(XorLHS.get());


Index: clang/test/SemaCXX/warn-xor-as-pow.cpp
===
--- clang/test/SemaCXX/warn-xor-as-pow.cpp
+++ clang/test/SemaCXX/warn-xor-as-pow.cpp
@@ -65,6 +65,7 @@
 
   res = 2 ^ 0x4;
   res = 2 ^ 04;
+  res = TWO ^ TEN;
   res = 0x2 ^ 10;
   res = 0X2 ^ 10;
   res = 02 ^ 10;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -12102,6 +12102,11 @@
   if (Loc.isMacroID())
 return;
 
+  // Do not diagnose if both LHS and RHS are macros
+  if (XorLHS.get()->getExprLoc().isMacroID() &&
+  XorRHS.get()->getExprLoc().isMacroID())
+return;
+
   bool Negative = false;
   bool ExplicitPlus = false;
   const auto *LHSInt = dyn_cast(XorLHS.get());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96944: [RecoveryAST] Add design doc to clang internal manual.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG77a8589e5d2f: [clang][RecoveryAST] Add design doc to clang 
internal manual. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96944

Files:
  clang/docs/InternalsManual.rst

Index: clang/docs/InternalsManual.rst
===
--- clang/docs/InternalsManual.rst
+++ clang/docs/InternalsManual.rst
@@ -1863,6 +1863,144 @@
 are not ``Redeclarable`` -- in that case, a ``Mergeable`` base class is used
 instead).
 
+Error Handling
+--
+
+Clang produces an AST even when the code contains errors. Clang won't generate
+and optimize code for it, but it's used as parsing continues to detect further
+errors in the input. Clang-based tools also depend on such ASTs, and IDEs in
+particular benefit from a high-quality AST for broken code.
+
+In presence of errors, clang uses a few error-recovery strategies to present the
+broken code in the AST:
+
+- correcting errors: in cases where clang is confident about the fix, it
+  provides a FixIt attaching to the error diagnostic and emits a corrected AST
+  (reflecting the written code with FixIts applied). The advantage of that is to
+  provide more accurate subsequent diagnostics. Typo correction is a typical
+  example.
+- representing invalid node: the invalid node is preserved in the AST in some
+  form, e.g. when the "declaration" part of the declaration contains semantic
+  errors, the Decl node is marked as invalid.
+- dropping invalid node: this often happens for errors that we don’t have
+  graceful recovery. Prior to Recovery AST, a mismatched-argument function call
+  expression was dropped though a CallExpr was created for semantic analysis.
+
+With these strategies, clang surfaces better diagnostics, and provides AST
+consumers a rich AST reflecting the written source code as much as possible even
+for broken code.
+
+Recovery AST
+
+
+The idea of Recovery AST is to use recovery nodes which act as a placeholder to
+maintain the rough structure of the parsing tree, preserve locations and
+children but have no language semantics attached to them.
+
+For example, consider the following mismatched function call:
+
+.. code-block:: c++
+
+   int NoArg();
+   void test(int abc) {
+ NoArg(abc); // oops, mismatched function arguments.
+   }
+
+Without Recovery AST, the invalid function call expression (and its child
+expressions) would be dropped in the AST:
+
+::
+
+|-FunctionDecl  NoArg 'int ()'
+`-FunctionDecl  test 'void (int)'
+ |-ParmVarDecl  col:15 used abc 'int'
+ `-CompoundStmt 
+
+
+With Recovery AST, the AST looks like:
+
+::
+
+|-FunctionDecl  NoArg 'int ()'
+`-FunctionDecl  test 'void (int)'
+  |-ParmVarDecl  used abc 'int'
+  `-CompoundStmt 
+`-RecoveryExpr  'int' contains-errors
+  |-UnresolvedLookupExpr  '' lvalue (ADL) = 'NoArg'
+  `-DeclRefExpr  'int' lvalue ParmVar 'abc' 'int'
+
+
+An alternative is to use existing Exprs, e.g. CallExpr for the above example.
+This would capture more call details (e.g. locations of parentheses) and allow
+it to be treated uniformly with valid CallExprs. However, jamming the data we
+have into CallExpr forces us to weaken its invariants, e.g. arg count may be
+wrong. This would introduce a huge burden on consumers of the AST to handle such
+"impossible" cases. So when we're representing (rather than correcting) errors,
+we use a distinct recovery node type with extremely weak invariants instead.
+
+``RecoveryExpr`` is the only recovery node so far. In practice, broken decls
+need more detailed semantics preserved (the current ``Invalid`` flag works
+fairly well), and completely broken statements with interesting internal
+structure are rare (so dropping the statements is OK).
+
+Types and dependence
+
+
+``RecoveryExpr`` is an ``Expr``, so it must have a type. In many cases the true
+type can't really be known until the code is corrected (e.g. a call to a
+function that doesn't exist). And it means that we can't properly perform type
+checks on some containing constructs, such as ``return 42 + unknownFunction()``.
+
+To model this, we generalize the concept of dependence from C++ templates to
+mean dependence on a template parameter or how an error is repaired. The
+``RecoveryExpr`` ``unknownFunction()`` has the totally unknown type
+``DependentTy``, and this suppresses type-based analysis in the same way it
+would inside a template.
+
+In cases where we are confident about the concrete type (e.g. the return type
+for a broken non-overloaded function call), the ``RecoveryExpr`` will have this
+type. This allows more code to be typechecked, and produces a better AST and
+more diagnostics. For example:
+
+.. code-block:: C++
+
+   unknownFunction().size() // .size() is a 

[clang] 77a8589 - [clang][RecoveryAST] Add design doc to clang internal manual.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2021-02-25T08:22:49+01:00
New Revision: 77a8589e5d2f1b3886a9831ae8468f97741991e7

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

LOG: [clang][RecoveryAST] Add design doc to clang internal manual.

Hopefully it would be useful for new developers.

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

Added: 


Modified: 
clang/docs/InternalsManual.rst

Removed: 




diff  --git a/clang/docs/InternalsManual.rst b/clang/docs/InternalsManual.rst
index 906b6d153f2b..da17d72906d7 100644
--- a/clang/docs/InternalsManual.rst
+++ b/clang/docs/InternalsManual.rst
@@ -1863,6 +1863,144 @@ or by simply a pointer to the canonical declaration (if 
the declarations
 are not ``Redeclarable`` -- in that case, a ``Mergeable`` base class is used
 instead).
 
+Error Handling
+--
+
+Clang produces an AST even when the code contains errors. Clang won't generate
+and optimize code for it, but it's used as parsing continues to detect further
+errors in the input. Clang-based tools also depend on such ASTs, and IDEs in
+particular benefit from a high-quality AST for broken code.
+
+In presence of errors, clang uses a few error-recovery strategies to present 
the
+broken code in the AST:
+
+- correcting errors: in cases where clang is confident about the fix, it
+  provides a FixIt attaching to the error diagnostic and emits a corrected AST
+  (reflecting the written code with FixIts applied). The advantage of that is 
to
+  provide more accurate subsequent diagnostics. Typo correction is a typical
+  example.
+- representing invalid node: the invalid node is preserved in the AST in some
+  form, e.g. when the "declaration" part of the declaration contains semantic
+  errors, the Decl node is marked as invalid.
+- dropping invalid node: this often happens for errors that we don’t have
+  graceful recovery. Prior to Recovery AST, a mismatched-argument function call
+  expression was dropped though a CallExpr was created for semantic analysis.
+
+With these strategies, clang surfaces better diagnostics, and provides AST
+consumers a rich AST reflecting the written source code as much as possible 
even
+for broken code.
+
+Recovery AST
+
+
+The idea of Recovery AST is to use recovery nodes which act as a placeholder to
+maintain the rough structure of the parsing tree, preserve locations and
+children but have no language semantics attached to them.
+
+For example, consider the following mismatched function call:
+
+.. code-block:: c++
+
+   int NoArg();
+   void test(int abc) {
+ NoArg(abc); // oops, mismatched function arguments.
+   }
+
+Without Recovery AST, the invalid function call expression (and its child
+expressions) would be dropped in the AST:
+
+::
+
+|-FunctionDecl  NoArg 'int ()'
+`-FunctionDecl  test 'void (int)'
+ |-ParmVarDecl  col:15 used abc 'int'
+ `-CompoundStmt 
+
+
+With Recovery AST, the AST looks like:
+
+::
+
+|-FunctionDecl  NoArg 'int ()'
+`-FunctionDecl  test 'void (int)'
+  |-ParmVarDecl  used abc 'int'
+  `-CompoundStmt 
+`-RecoveryExpr  'int' contains-errors
+  |-UnresolvedLookupExpr  '' lvalue 
(ADL) = 'NoArg'
+  `-DeclRefExpr  'int' lvalue ParmVar 'abc' 'int'
+
+
+An alternative is to use existing Exprs, e.g. CallExpr for the above example.
+This would capture more call details (e.g. locations of parentheses) and allow
+it to be treated uniformly with valid CallExprs. However, jamming the data we
+have into CallExpr forces us to weaken its invariants, e.g. arg count may be
+wrong. This would introduce a huge burden on consumers of the AST to handle 
such
+"impossible" cases. So when we're representing (rather than correcting) errors,
+we use a distinct recovery node type with extremely weak invariants instead.
+
+``RecoveryExpr`` is the only recovery node so far. In practice, broken decls
+need more detailed semantics preserved (the current ``Invalid`` flag works
+fairly well), and completely broken statements with interesting internal
+structure are rare (so dropping the statements is OK).
+
+Types and dependence
+
+
+``RecoveryExpr`` is an ``Expr``, so it must have a type. In many cases the true
+type can't really be known until the code is corrected (e.g. a call to a
+function that doesn't exist). And it means that we can't properly perform type
+checks on some containing constructs, such as ``return 42 + 
unknownFunction()``.
+
+To model this, we generalize the concept of dependence from C++ templates to
+mean dependence on a template parameter or how an error is repaired. The
+``RecoveryExpr`` ``unknownFunction()`` has the totally unknown type
+``DependentTy``, and this suppresses type-based analysis in the same way it
+would inside a 

[PATCH] D96203: [clang][patch] Modify sanitizer options names: renaming blacklist to blocklist

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

Could you please rebase the patch onto D96974 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96203

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

[PATCH] D97273: OpenMP: Fix object clobbering issue when using save-temps

[Pushpinder Singh via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG99951aa68da3: OpenMP: Fix object clobbering issue when using 
save-temps (authored by pdhaliwal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97273

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/openmp-offload-gpu.c


Index: clang/test/Driver/openmp-offload-gpu.c
===
--- clang/test/Driver/openmp-offload-gpu.c
+++ clang/test/Driver/openmp-offload-gpu.c
@@ -310,3 +310,9 @@
 // RUN:   | FileCheck -check-prefix=OPENMP_NVPTX_WRAPPERS %s
 // OPENMP_NVPTX_WRAPPERS: clang{{.*}}"-cc1"{{.*}}"-triple" 
"nvptx64-nvidia-cuda"
 // OPENMP_NVPTX_WRAPPERS-SAME: "-internal-isystem" "{{.*}}openmp_wrappers"
+
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN:  -save-temps -no-canonical-prefixes -ccc-print-bindings %s -o 
openmp-offload-gpu 2>&1 \
+// RUN:   | FileCheck -check-prefix=SAVE_TEMPS_NAMES %s
+
+// SAVE_TEMPS_NAMES-NOT: "GNU::Linker"{{.*}}["[[SAVE_TEMPS_INPUT1:.*\.o]]", 
"[[SAVE_TEMPS_INPUT1]]"]
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4675,11 +4675,12 @@
 /*CreatePrefixForHost=*/!!A->getOffloadingHostActiveKinds() &&
 !AtTopLevel);
 if (isa(JA)) {
-  OffloadingPrefix += "-wrapper";
   if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))
 BaseInput = FinalOutput->getValue();
   else
 BaseInput = getDefaultImageName();
+  BaseInput =
+  C.getArgs().MakeArgString(std::string(BaseInput) + "-wrapper");
 }
 Result = InputInfo(A, GetNamedOutputPath(C, *JA, BaseInput, BoundArch,
  AtTopLevel, MultipleArchs,


Index: clang/test/Driver/openmp-offload-gpu.c
===
--- clang/test/Driver/openmp-offload-gpu.c
+++ clang/test/Driver/openmp-offload-gpu.c
@@ -310,3 +310,9 @@
 // RUN:   | FileCheck -check-prefix=OPENMP_NVPTX_WRAPPERS %s
 // OPENMP_NVPTX_WRAPPERS: clang{{.*}}"-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"
 // OPENMP_NVPTX_WRAPPERS-SAME: "-internal-isystem" "{{.*}}openmp_wrappers"
+
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN:  -save-temps -no-canonical-prefixes -ccc-print-bindings %s -o openmp-offload-gpu 2>&1 \
+// RUN:   | FileCheck -check-prefix=SAVE_TEMPS_NAMES %s
+
+// SAVE_TEMPS_NAMES-NOT: "GNU::Linker"{{.*}}["[[SAVE_TEMPS_INPUT1:.*\.o]]", "[[SAVE_TEMPS_INPUT1]]"]
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4675,11 +4675,12 @@
 /*CreatePrefixForHost=*/!!A->getOffloadingHostActiveKinds() &&
 !AtTopLevel);
 if (isa(JA)) {
-  OffloadingPrefix += "-wrapper";
   if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))
 BaseInput = FinalOutput->getValue();
   else
 BaseInput = getDefaultImageName();
+  BaseInput =
+  C.getArgs().MakeArgString(std::string(BaseInput) + "-wrapper");
 }
 Result = InputInfo(A, GetNamedOutputPath(C, *JA, BaseInput, BoundArch,
  AtTopLevel, MultipleArchs,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 99951aa - OpenMP: Fix object clobbering issue when using save-temps

[Pushpinder Singh via cfe-commits]

Author: Pushpinder Singh
Date: 2021-02-25T00:50:51-05:00
New Revision: 99951aa68da3c85ba03edf977cd9b22458aae6ca

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

LOG: OpenMP: Fix object clobbering issue when using save-temps

There are two preconditions to reproduce the issue,
 1. Use -save-temps option
 2. Provide the -o option with name equal to the input file name
without the file extension. For e.g. clang a.c -o a

With the -o specified, the AssembleJobAction after OffloadWrapperJobAction
will produce the object file with same name as host code object file.
Due to this clash, the OffloadWrapperAction overwrites the initial host
object file, which results in lld error. This also fixes the `multiple 
definition of __dummy.omp_offloading.entry'` issue in D96769 .

Reviewed By: jdoerfert

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/test/Driver/openmp-offload-gpu.c

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 566fd63e8478..8c180140ae92 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4675,11 +4675,12 @@ InputInfo Driver::BuildJobsForActionNoCache(
 /*CreatePrefixForHost=*/!!A->getOffloadingHostActiveKinds() &&
 !AtTopLevel);
 if (isa(JA)) {
-  OffloadingPrefix += "-wrapper";
   if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))
 BaseInput = FinalOutput->getValue();
   else
 BaseInput = getDefaultImageName();
+  BaseInput =
+  C.getArgs().MakeArgString(std::string(BaseInput) + "-wrapper");
 }
 Result = InputInfo(A, GetNamedOutputPath(C, *JA, BaseInput, BoundArch,
  AtTopLevel, MultipleArchs,

diff  --git a/clang/test/Driver/openmp-offload-gpu.c 
b/clang/test/Driver/openmp-offload-gpu.c
index 37de504bfe73..f8f063503a9e 100644
--- a/clang/test/Driver/openmp-offload-gpu.c
+++ b/clang/test/Driver/openmp-offload-gpu.c
@@ -310,3 +310,9 @@
 // RUN:   | FileCheck -check-prefix=OPENMP_NVPTX_WRAPPERS %s
 // OPENMP_NVPTX_WRAPPERS: clang{{.*}}"-cc1"{{.*}}"-triple" 
"nvptx64-nvidia-cuda"
 // OPENMP_NVPTX_WRAPPERS-SAME: "-internal-isystem" "{{.*}}openmp_wrappers"
+
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN:  -save-temps -no-canonical-prefixes -ccc-print-bindings %s -o 
openmp-offload-gpu 2>&1 \
+// RUN:   | FileCheck -check-prefix=SAVE_TEMPS_NAMES %s
+
+// SAVE_TEMPS_NAMES-NOT: "GNU::Linker"{{.*}}["[[SAVE_TEMPS_INPUT1:.*\.o]]", 
"[[SAVE_TEMPS_INPUT1]]"]



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

[PATCH] D95984: [CodeGen] Fix codegen for __attribute__((swiftasynccall)).

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Please add some C++ tests, just in case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95984

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

[PATCH] D95561: [Clang] Introduce Swift async calling convention.

[John McCall via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95561

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

[PATCH] D95984: [CodeGen] Fix codegen for __attribute__((swiftasynccall)).

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay, seems fine to me.




Comment at: clang/lib/CodeGen/CGStmt.cpp:1156
+  CallingConv::CC_SwiftAsync)) {
+auto CI = cast(()->back());
+CI->setTailCallKind(llvm::CallInst::TCK_Tail);

Hmm.  I guess this should work in any situation where we can successfully 
actually do a tail call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95984

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

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

not exactly sure how to test this...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97437

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

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks created this revision.
aeubanks added reviewers: rnk, thakis, amccarth.
Herald added subscribers: usaxena95, kadircet.
aeubanks requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.

This fixes an issue where the toolchain discovery doesn't respect the
VFS's current working directory, specifically clangd not respecting a
relative /winsysroot.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97437

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

Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include 
 
 #ifdef _WIN32
@@ -61,19 +62,29 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static bool canExecute(llvm::vfs::FileSystem , StringRef Path) {
+  auto Status = VFS.status(Path);
+  if (!Status)
+return false;
+  return (Status->getPermissions() & llvm::sys::fs::perms::all_exe) != 0;
+}
+
 // Defined below.
 // Forward declare this so there aren't too many things above the constructor.
 static bool getSystemRegistryString(const char *keyPath, const char *valueName,
 std::string , std::string *phValue);
 
-static std::string getHighestNumericTupleInDirectory(StringRef Directory) {
+static std::string getHighestNumericTupleInDirectory(llvm::vfs::FileSystem ,
+ StringRef Directory) {
   std::string Highest;
   llvm::VersionTuple HighestTuple;
 
   std::error_code EC;
-  for (llvm::sys::fs::directory_iterator DirIt(Directory, EC), DirEnd;
+  for (llvm::vfs::directory_iterator DirIt = VFS.dir_begin(Directory, EC),
+ DirEnd;
!EC && DirIt != DirEnd; DirIt.increment(EC)) {
-if (!llvm::sys::fs::is_directory(DirIt->path()))
+auto Status = VFS.status(DirIt->path());
+if (!Status || !Status->isDirectory())
   continue;
 StringRef CandidateName = llvm::sys::path::filename(DirIt->path());
 llvm::VersionTuple Tuple;
@@ -90,7 +101,8 @@
 
 // Check command line arguments to try and find a toolchain.
 static bool
-findVCToolChainViaCommandLine(const ArgList , std::string ,
+findVCToolChainViaCommandLine(llvm::vfs::FileSystem , const ArgList ,
+  std::string ,
   MSVCToolChain::ToolsetLayout ) {
   // Don't validate the input; trust the value supplied by the user.
   // The primary motivation is to prevent unnecessary file and registry access.
@@ -103,7 +115,7 @@
   if (Arg *A = Args.getLastArg(options::OPT__SLASH_vctoolsversion))
 VCToolsVersion = A->getValue();
   else
-VCToolsVersion = getHighestNumericTupleInDirectory(ToolsPath);
+VCToolsVersion = getHighestNumericTupleInDirectory(VFS, ToolsPath);
   llvm::sys::path::append(ToolsPath, VCToolsVersion);
   Path = std::string(ToolsPath.str());
 } else {
@@ -117,7 +129,7 @@
 
 // Check various environment variables to try and find a toolchain.
 static bool
-findVCToolChainViaEnvironment(std::string ,
+findVCToolChainViaEnvironment(llvm::vfs::FileSystem , std::string ,
   MSVCToolChain::ToolsetLayout ) {
   // These variables are typically set by vcvarsall.bat
   // when launching a developer command prompt.
@@ -156,14 +168,14 @@
   // If cl.exe doesn't exist, then this definitely isn't a VC toolchain.
   ExeTestPath = PathEntry;
   llvm::sys::path::append(ExeTestPath, "cl.exe");
-  if (!llvm::sys::fs::exists(ExeTestPath))
+  if (!VFS.exists(ExeTestPath))
 continue;
 
   // cl.exe existing isn't a conclusive test for a VC toolchain; clang also
   // has a cl.exe. So let's check for link.exe too.
   ExeTestPath = PathEntry;
   llvm::sys::path::append(ExeTestPath, "link.exe");
-  if (!llvm::sys::fs::exists(ExeTestPath))
+  if (!VFS.exists(ExeTestPath))
 continue;
 
   // whatever/VC/bin --> old toolchain, VC dir is toolchain dir.
@@ -229,8 +241,9 @@
 // and find its default VC toolchain.
 // This is the preferred way to discover new Visual Studios, as they're no
 // longer listed in the registry.
-static bool findVCToolChainViaSetupConfig(std::string ,
-  MSVCToolChain::ToolsetLayout ) {
+static bool
+findVCToolChainViaSetupConfig(llvm::vfs::FileSystem , std::string ,
+  MSVCToolChain::ToolsetLayout ) {
 #if !defined(USE_MSVC_SETUP_API)
   return false;
 #else
@@ -308,7 +321,8 @@
   llvm::SmallString<256> ToolchainPath(VCRootPath);
   llvm::sys::path::append(ToolchainPath, "Tools", "MSVC",
   

[PATCH] D97433: [Driver] Create -ffile-compilation-dir alias

[Fangrui Song via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97433

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

[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:311
+  Value *ResElt = B.CreateAdd(EltC, SubVecR);
+  Value *NewVecC = B.CreateInsertElement(VecCPhi, ResElt, IdxC);
+  Value *NewVecD = B.CreateInsertElement(VecDPhi, ResElt, IdxC);

yubing wrote:
> pengfei wrote:
> > Is it necessary to insert the ResElt to VecC?
> Yes, it is necessary since you should use updated eltC(aka, Cij) when you are 
> doing matrix dotproduct:
> Cij =Cij+Ai1.*B1j
> Cij =Cij+Ai2.*B2j
> 
> Cij =Cij+AiK.*BKj
But you don't need to update both C and D. Something like the psudo code should 
enough:
```
for (k : K)
  Dij += Aik * Bkj;
Dij += Cij
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93594

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

[PATCH] D97433: [Driver] Create -ffile-compilation-dir alias

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 326269.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97433

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/clang_f_opts.c
  clang/tools/driver/cc1as_main.cpp


Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -240,8 +240,9 @@
   std::string(Args.getLastArgValue(OPT_dwarf_debug_flags));
   Opts.DwarfDebugProducer =
   std::string(Args.getLastArgValue(OPT_dwarf_debug_producer));
-  Opts.DebugCompilationDir =
-  std::string(Args.getLastArgValue(OPT_fdebug_compilation_dir));
+  if (const Arg *A = Args.getLastArg(options::OPT_ffile_compilation_dir_EQ,
+ options::OPT_fdebug_compilation_dir_EQ))
+Opts.DebugCompilationDir = A->getValue();
   Opts.MainFileName = std::string(Args.getLastArgValue(OPT_main_file_name));
 
   for (const auto  : Args.getAllArgValues(OPT_fdebug_prefix_map_EQ)) {
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -504,8 +504,13 @@
 // RUN: %clang -### -S -fdebug-compilation-dir=. %s 2>&1 | FileCheck 
-check-prefix=CHECK-DEBUG-COMPILATION-DIR %s
 // RUN: %clang -### -fdebug-compilation-dir . -x assembler %s 2>&1 | FileCheck 
-check-prefix=CHECK-DEBUG-COMPILATION-DIR %s
 // RUN: %clang -### -fdebug-compilation-dir=. -x assembler %s 2>&1 | FileCheck 
-check-prefix=CHECK-DEBUG-COMPILATION-DIR %s
+// RUN: %clang -### -ffile-compilation-dir=. -x assembler %s 2>&1 | FileCheck 
-check-prefix=CHECK-FILE-COMPILATION-DIR %s
 // CHECK-DEBUG-COMPILATION-DIR: "-fdebug-compilation-dir=."
 
+// RUN: %clang -### -S -ffile-compilation-dir=. %s 2>&1 | FileCheck 
-check-prefix=CHECK-FILE-COMPILATION-DIR %s
+// RUN: %clang -### -ffile-compilation-dir=. -x assembler %s 2>&1 | FileCheck 
-check-prefix=CHECK-FILE-COMPILATION-DIR %s
+// CHECK-FILE-COMPILATION-DIR: "-ffile-compilation-dir=."
+
 // RUN: %clang -### -S -fdiscard-value-names %s 2>&1 | FileCheck 
-check-prefix=CHECK-DISCARD-NAMES %s
 // RUN: %clang -### -S -fno-discard-value-names %s 2>&1 | FileCheck 
-check-prefix=CHECK-NO-DISCARD-NAMES %s
 // CHECK-DISCARD-NAMES: "-discard-value-names"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1010,12 +1010,13 @@
 return Args.MakeArgString(T);
   } else {
 // Use the compilation dir.
-SmallString<128> T(
-Args.getLastArgValue(options::OPT_fdebug_compilation_dir_EQ));
+Arg *A = Args.getLastArg(options::OPT_ffile_compilation_dir_EQ,
+ options::OPT_fdebug_compilation_dir_EQ);
+SmallString<128> T(A ? A->getValue() : "");
 SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput()));
 AddPostfix(F);
 T += F;
-return Args.MakeArgString(F);
+return Args.MakeArgString(T);
   }
 }
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -618,7 +618,8 @@
 /// Add a CC1 option to specify the debug compilation directory.
 static void addDebugCompDirArg(const ArgList , ArgStringList ,
const llvm::vfs::FileSystem ) {
-  if (Arg *A = Args.getLastArg(options::OPT_fdebug_compilation_dir_EQ)) {
+  if (Arg *A = Args.getLastArg(options::OPT_ffile_compilation_dir_EQ,
+   options::OPT_fdebug_compilation_dir_EQ)) {
 A->render(Args, CmdArgs);
   } else if (llvm::ErrorOr CWD =
  VFS.getCurrentWorkingDirectory()) {
@@ -859,7 +860,8 @@
 CmdArgs.push_back("-fcoverage-mapping");
   }
 
-  if (Arg *A = Args.getLastArg(options::OPT_fprofile_compilation_dir_EQ)) {
+  if (Arg *A = Args.getLastArg(options::OPT_ffile_compilation_dir_EQ,
+   options::OPT_fprofile_compilation_dir_EQ)) {
 A->render(Args, CmdArgs);
   } else if (llvm::ErrorOr CWD =
  D.getVFS().getCurrentWorkingDirectory()) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1112,6 +1112,8 @@
 Group, Flags<[CC1Option, CC1AsOption, CoreOption]>,
 HelpText<"The compilation directory to embed in the coverage mapping.">,
 MarshallingInfoString>;
+def ffile_compilation_dir_EQ : Joined<["-"], "ffile-compilation-dir=">, 
Group,
+HelpText<"The compilation directory to 

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added inline comments.



Comment at: llvm/lib/Transforms/Utils/RelLookupTableGenerator.cpp:76-77
+
+  /// If lookup table has more than one user,
+  /// do not generate a relative lookup table.
+  if (!GlobalVar.hasOneUser())

leonardchan wrote:
> What's the reason for why the number of users matters?
We generate one lookup table per switch statement whenever possible.
I think there is only one use of that lookup table which is the`GetElementPtr` 
instruction.
That is why I checked for one use.
Do you see any issue with that?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

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

[PATCH] D97434: [Driver] Rename -fprofile-{prefix-map,compilation-dir} to -fcoverage-{prefix-map,compilation-dir}

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

I realized recently that `-fcoverage-{prefix-map,compilation-dir}` is a more 
accurate name but I'd be interested in your opinion, I don't feel too strongly 
about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97434

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

[PATCH] D97434: [Driver] Rename -fprofile-{prefix-map,compilation-dir} to -fcoverage-{prefix-map,compilation-dir}

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: vsk, davidxl, keith.
Herald added subscribers: jansvoboda11, dexonsmith, dang.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These flags affect coverage mapping (-fcoverage-mapping), not
-fprofile-[instr-]generate so it makes more sense to use the
-fcoverage-* prefix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97434

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/coverage-compilation-dir.c
  clang/test/CodeGen/profile-compilation-dir.c
  clang/test/Driver/debug-prefix-map.c
  clang/test/Profile/coverage-prefix-map.c
  clang/test/Profile/profile-prefix-map.c

Index: clang/test/Profile/profile-prefix-map.c
===
--- clang/test/Profile/profile-prefix-map.c
+++ /dev/null
@@ -1,21 +0,0 @@
-// %s expands to an absolute path, so to test relative paths we need to create a
-// clean directory, put the source there, and cd into it.
-// RUN: rm -rf %t
-// RUN: mkdir -p %t/root/nested
-// RUN: echo "void f1() {}" > %t/root/nested/profile-prefix-map.c
-// RUN: cd %t/root
-
-// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c %t/root/nested/profile-prefix-map.c -o - | FileCheck --check-prefix=ABSOLUTE %s
-//
-// ABSOLUTE: @__llvm_coverage_mapping = {{.*"\\02.*root.*nested.*profile-prefix-map\.c}}
-
-// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c ../root/nested/profile-prefix-map.c -o - | FileCheck --check-prefix=RELATIVE %s
-//
-// RELATIVE: @__llvm_coverage_mapping = {{.*"\\02.*}}..{{/|\\+}}root{{/|\\+}}nested{{.*profile-prefix-map\.c}}
-
-// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c %t/root/nested/profile-prefix-map.c -fprofile-prefix-map=%/t/root=. -o - | FileCheck --check-prefix=PROFILE-PREFIX-MAP %s
-// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c ../root/nested/profile-prefix-map.c -fprofile-prefix-map=../root=. -o - | FileCheck --check-prefix=PROFILE-PREFIX-MAP %s
-// PROFILE-PREFIX-MAP: @__llvm_coverage_mapping = {{.*"\\02.*}}.{{/|\\+}}nested{{.*profile-prefix-map\.c}}
-
-// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c %t/root/nested/profile-prefix-map.c -fprofile-compilation-dir=/custom -fprofile-prefix-map=/custom=/nonsense -o - | FileCheck --check-prefix=PROFILE-COMPILATION-DIR %s
-// PROFILE-COMPILATION-DIR: @__llvm_coverage_mapping = {{.*"\\02.*}}nonsense
Index: clang/test/Profile/coverage-prefix-map.c
===
--- /dev/null
+++ clang/test/Profile/coverage-prefix-map.c
@@ -0,0 +1,21 @@
+// %s expands to an absolute path, so to test relative paths we need to create a
+// clean directory, put the source there, and cd into it.
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/root/nested
+// RUN: echo "void f1() {}" > %t/root/nested/coverage-prefix-map.c
+// RUN: cd %t/root
+
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name coverage-prefix-map.c %t/root/nested/coverage-prefix-map.c -o - | FileCheck --check-prefix=ABSOLUTE %s
+//
+// ABSOLUTE: @__llvm_coverage_mapping = {{.*"\\02.*root.*nested.*coverage-prefix-map\.c}}
+
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name coverage-prefix-map.c ../root/nested/coverage-prefix-map.c -o - | FileCheck --check-prefix=RELATIVE %s
+//
+// RELATIVE: @__llvm_coverage_mapping = {{.*"\\02.*}}..{{/|\\+}}root{{/|\\+}}nested{{.*coverage-prefix-map\.c}}
+
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name coverage-prefix-map.c %t/root/nested/coverage-prefix-map.c -fcoverage-prefix-map=%/t/root=. -o - | FileCheck --check-prefix=COVERAGE-PREFIX-MAP %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name coverage-prefix-map.c ../root/nested/coverage-prefix-map.c -fcoverage-prefix-map=../root=. -o - | FileCheck --check-prefix=COVERAGE-PREFIX-MAP %s
+// COVERAGE-PREFIX-MAP: @__llvm_coverage_mapping = {{.*"\\02.*}}.{{/|\\+}}nested{{.*coverage-prefix-map\.c}}
+
+// RUN: %clang_cc1 

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added a comment.

In D94355#2572553 , @leonardchan wrote:

> It looks like you have everything setup for running on the new PM, but it 
> doesn't look like the pass is added anywhere in the new PM pipeline.

Thank you very much for pointing that out @leonardchan ! 
I added this pass into both pass managers now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

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

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added a comment.

> Thanks for pushing this forward! I think this will be a nice transformation 
> once all the details are worked out.

Thank you very much for all of your wonderful constructive feedback!
I learned much more about LLVM and IR internals. 
Appreciate all your help!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

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

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem marked 17 inline comments as done.
gulfem added inline comments.



Comment at: clang/test/CodeGen/switch-to-lookup-table.c:2
+// Check switch to lookup optimization in fPIC and fno-PIC mode
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -O2 -fno-PIC -S 
-emit-llvm -o - | FileCheck %s --check-prefix=CHECK --check-prefix=FNOPIC
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -O2 -fPIC -S -emit-llvm 
-o - | FileCheck %s --check-prefix=CHECK --check-prefix=FPIC

hans wrote:
> gulfem wrote:
> > hans wrote:
> > > Clang codegen tests are not normally used to test LLVM optimizations. I 
> > > think the tests for this should all live in LLVM, not Clang.
> > > 
> > > (Also aarch64 is not guaranteed to be included as a target in the LLVM 
> > > build, so this test would not necessarily run.)
> > I'm not able to use -fPIC and -fno-PIC options in the `opt` tool.
> > I am setting the `PIC Level` flag to enable -fPIC in `opt.
> > I thought that testing -fPIC and -fno-PIC in the same file seems easier and 
> > more readable  in CodeGen tests.
> > Please let me know if you have a good suggestion how to do that with `opt`.
> > 
> > I changed the target to `x86_64-linux` in this test.
> Buildbots may not have x86_64 as a registered target, so this test will break 
> some buildbots.
> 
> I think the opt flags -relocation-model=pic and -relocation-model=static will 
> do the right thing (see for example 
> llvm/test/Transforms/SimplifyCFG/ARM/switch-to-lookup-table.ll
I added `x86-registered-target` to ensure that it only runs on buildbots that 
have `x86_64` as a registered target



Comment at: llvm/lib/Transforms/Utils/RelLookupTableGenerator.cpp:39
+
+  // If not in PIC mode, do not generate a relative lookup table.
+  if (M.getPICLevel() == PICLevel::NotPIC)

hans wrote:
> Again, this needs the "why".
> And perhaps it would make sense to put this check first.
I added the reason, please let me know if that's not clear enough.



Comment at: llvm/lib/Transforms/Utils/RelLookupTableGenerator.cpp:169
+
+  for (Module::global_iterator GVI = M.global_begin(), E = M.global_end();
+   GVI != E;) {

hans wrote:
> This would be simpler as
> 
> ```
> for (GlobalVariable *GlobalVar : M.globals()) {
> ```
Please keep that in mind that I'm deleting a global variable while I'm 
iterating over global variables.
I don't want to have invalidated iterator, and this is why I did not use a 
simple for loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

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

[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

[Bing Yu via Phabricator via cfe-commits]

yubing added inline comments.



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:311
+  Value *ResElt = B.CreateAdd(EltC, SubVecR);
+  Value *NewVecC = B.CreateInsertElement(VecCPhi, ResElt, IdxC);
+  Value *NewVecD = B.CreateInsertElement(VecDPhi, ResElt, IdxC);

pengfei wrote:
> Is it necessary to insert the ResElt to VecC?
Yes, it is necessary since you should use updated eltC(aka, Cij) when you are 
doing matrix dotproduct:
Cij =Cij+Ai1.*B1j
Cij =Cij+Ai2.*B2j

Cij =Cij+AiK.*BKj


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93594

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

[PATCH] D94355: [Passes] Add relative lookup table generator pass

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem updated this revision to Diff 326265.
gulfem added a comment.
Herald added subscribers: wenlei, nikic, kerbowa, steven_wu, nhaehnle, jvesely.

- Rename the pass to RelLookupTableConverter to be consistent
- Addressed reviewers' feedback
- Added tests for user-defined lookup tables and hidden visibility


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

Files:
  clang/test/CodeGen/switch-to-lookup-table.c
  llvm/docs/Passes.rst
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Scalar.h
  llvm/include/llvm/Transforms/Utils/RelLookupTableConverter.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp
  llvm/lib/Transforms/Utils/Utils.cpp
  llvm/test/CodeGen/AMDGPU/opt-pipeline.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/Transforms/SimplifyCFG/X86/relative_lookup_table.ll
  llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn
@@ -60,6 +60,7 @@
 "NameAnonGlobals.cpp",
 "PredicateInfo.cpp",
 "PromoteMemoryToRegister.cpp",
+"RelLookupTableConverter.cpp"
 "SSAUpdater.cpp",
 "SSAUpdaterBulk.cpp",
 "SampleProfileLoaderBaseUtil.cpp",
Index: llvm/test/Transforms/SimplifyCFG/X86/relative_lookup_table.ll
===
--- /dev/null
+++ llvm/test/Transforms/SimplifyCFG/X86/relative_lookup_table.ll
@@ -0,0 +1,310 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -rel-lookup-table-converter -S | FileCheck %s
+; RUN: opt < %s -passes=rel-lookup-table-converter -S | FileCheck %s
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str = private unnamed_addr constant [5 x i8] c"zero\00", align 1
+@.str.1 = private unnamed_addr constant [4 x i8] c"one\00", align 1
+@.str.2 = private unnamed_addr constant [4 x i8] c"two\00", align 1
+@.str.3 = private unnamed_addr constant [8 x i8] c"default\00", align 1
+@.str.4 = private unnamed_addr constant [6 x i8] c"three\00", align 1
+@.str.5 = private unnamed_addr constant [5 x i8] c"str1\00", align 1
+@.str.6 = private unnamed_addr constant [5 x i8] c"str2\00", align 1
+@.str.7 = private unnamed_addr constant [12 x i8] c"singlevalue\00", align 1
+
+@a1 = external global i32, align 4
+@b1 = external global i32, align 4
+@c1 = external global i32, align 4
+@d1 = external global i32, align 4
+
+@a2 = internal global i32 0, align 4
+@b2 = internal global i32 0, align 4
+@c2 = internal global i32 0, align 4
+@d2 = internal global i32 0, align 4
+
+@hidden0 = external hidden global i32, align 8
+@hidden1 = external hidden global i32, align 8
+@hidden2 = external hidden global i32, align 8
+@hidden3 = external hidden global i32, align 8
+
+@switch.table.no_dso_local = private unnamed_addr constant [3 x i32*] [i32* @a1, i32* @b1, i32* @c1], align 8
+
+@switch.table.dso_local = private unnamed_addr constant [3 x i32*] [i32* @a2, i32* @b2, i32* @c2], align 8
+
+@switch.table.hidden = private unnamed_addr constant [3 x i32*] [i32* @hidden0, i32* @hidden1, i32* @hidden2], align 8
+
+@switch.table.string_table = private unnamed_addr constant [3 x i8*]
+ [
+  i8* getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, i64 0),
+  i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.1, i64 0, i64 0),
+  i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.2, i64 0, i64 0)
+ ], align 8
+
+@switch.table.string_table_holes = private unnamed_addr constant [4 x i8*]
+   [
+i8* getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, i64 0),
+i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str.3, i64 0, i64 0),
+i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.2, i64 0, i64 0),
+i8* getelementptr inbounds ([6 x i8], [6 x i8]* 

[PATCH] D94640: adds more checks to -Wfree-nonheap-object

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 326261.
cjdb added a comment.

applies @aaron.ballman's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94640

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Analysis/free.c
  clang/test/Analysis/free.cpp
  clang/test/Analysis/malloc-fnptr-plist.c
  clang/test/Analysis/malloc.c
  clang/test/Analysis/weak-functions.c

Index: clang/test/Analysis/weak-functions.c
===
--- clang/test/Analysis/weak-functions.c
+++ clang/test/Analysis/weak-functions.c
@@ -71,7 +71,9 @@
 void free(void *) __attribute__((weak_import));
 
 void t10 () {
-  free((void*)); // expected-warning {{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}}
+  free((void*));
+  // expected-warning@-1{{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 't10'}}
 }
 
 //===--===
Index: clang/test/Analysis/malloc.c
===
--- clang/test/Analysis/malloc.c
+++ clang/test/Analysis/malloc.c
@@ -1780,7 +1780,9 @@
 }
 
 void freeFunctionPtr() {
-  free((void *)fnptr); // expected-warning {{Argument to free() is a function pointer}}
+  free((void *)fnptr);
+  // expected-warning@-1{{Argument to free() is a function pointer}}
+  // expected-warning@-2{{attempt to call free on non-heap object '(void *)fnptr'}}
 }
 
 void allocateSomeMemory(void *offendingParameter, void **ptr) {
Index: clang/test/Analysis/malloc-fnptr-plist.c
===
--- clang/test/Analysis/malloc-fnptr-plist.c
+++ clang/test/Analysis/malloc-fnptr-plist.c
@@ -4,7 +4,9 @@
 void free(void *);
 void (*fnptr)(int);
 void foo() {
-  free((void *)fnptr); // expected-warning{{Argument to free() is a function pointer}}
+  free((void *)fnptr);
+  // expected-warning@-1{{Argument to free() is a function pointer}}
+  // expected-warning@-2{{attempt to call free on non-heap object '(void *)fnptr'}}
 }
 
 // Make sure the bug category is correct.
Index: clang/test/Analysis/free.cpp
===
--- /dev/null
+++ clang/test/Analysis/free.cpp
@@ -0,0 +1,210 @@
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-store=region \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix.Malloc
+//
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-store=region \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
+namespace std {
+  using size_t = decltype(sizeof(int));
+  void free(void *);
+}
+
+extern "C" void free(void *);
+extern "C" void *alloca(std::size_t);
+
+void t1a () {
+  int a[] = { 1 };
+  free(a);
+  // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 'a'}}
+}
+
+void t1b () {
+  int a[] = { 1 };
+  std::free(a);
+  // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object 'a'}}
+}
+
+void t2a () {
+  int a = 1;
+  free();
+  // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 'a'}}
+}
+
+void t2b () {
+  int a = 1;
+  std::free();
+  // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object 'a'}}
+}
+
+void t3a () {
+  static int a[] = { 1 };
+  free(a);
+  // expected-warning@-1{{Argument to free() is the address of the static variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call free on non-heap object 'a'}}
+}
+
+void t3b () {
+  static int a[] = { 1 };
+  std::free(a);
+  // expected-warning@-1{{Argument to free() is the address of the static variable 'a', which is not memory allocated by malloc()}}
+  // expected-warning@-2{{attempt to call std::free on non-heap object 'a'}}
+}
+
+void t4a (char *x) {
+  free(x); // no-warning
+}
+
+void t4b (char *x) {
+  std::free(x); // no-warning
+}
+
+void t5a () {
+  extern char *ptr();
+  free(ptr()); // no-warning
+}
+
+void t5b () {
+  extern char *ptr();
+  std::free(ptr()); // no-warning
+}
+
+void t6a () {
+  free((void*)1000);
+  // expected-warning@-1{{Argument 

[PATCH] D97364: [docs] Add a release note for the removing of -Wreturn-std-move-in-c++11

[Yang Fan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb950de5c13ef: [docs] Add a release note for the removing of 
-Wreturn-std-move-in-c++11 (authored by nullptr.cpp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97364

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -79,6 +79,10 @@
 - The clang-cl ``/fallback`` flag, which made clang-cl invoke Microsoft Visual
   C++ on files it couldn't compile itself, has been removed.
 
+- ``-Wreturn-std-move-in-c++11``, which checked whether an entity is affected 
by
+  `CWG1579 `_ to become implicitly movable, has been
+  removed.
+
 New Pragmas in Clang
 
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -79,6 +79,10 @@
 - The clang-cl ``/fallback`` flag, which made clang-cl invoke Microsoft Visual
   C++ on files it couldn't compile itself, has been removed.
 
+- ``-Wreturn-std-move-in-c++11``, which checked whether an entity is affected by
+  `CWG1579 `_ to become implicitly movable, has been
+  removed.
+
 New Pragmas in Clang
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b950de5 - [docs] Add a release note for the removing of -Wreturn-std-move-in-c++11

[Yang Fan via cfe-commits]

Author: Yang Fan
Date: 2021-02-25T10:17:09+08:00
New Revision: b950de5c13ef2171c0457a413c3b06aa31047938

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

LOG: [docs] Add a release note for the removing of -Wreturn-std-move-in-c++11

`-Wreturn-std-move-in-c++11` has been removed in 
fbee4a0c79cc4ee87c34e51342742a5bc6fcf872.

Reviewed By: aaronpuchert, amccarth

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 91d06940748e..26b380407c0b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -79,6 +79,10 @@ Removed Compiler Flags
 - The clang-cl ``/fallback`` flag, which made clang-cl invoke Microsoft Visual
   C++ on files it couldn't compile itself, has been removed.
 
+- ``-Wreturn-std-move-in-c++11``, which checked whether an entity is affected 
by
+  `CWG1579 `_ to become implicitly movable, has been
+  removed.
+
 New Pragmas in Clang
 
 



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

[PATCH] D97364: [docs] Add a release note for the removing of -Wreturn-std-move-in-c++11

[Yang Fan via Phabricator via cfe-commits]

nullptr.cpp added a comment.

In D97364#2586171 , @aaronpuchert 
wrote:

> I assume you need someone to land this for you?

I can do it by myself, I have the commit access now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97364

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

[PATCH] D97433: [Driver] Create -ffile-compilation-dir alias

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: MaskRay, rnk.
Herald added subscribers: jansvoboda11, dang.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We introduce -ffile-compilation-dir shorthand to avoid having to set
-fdebug-compilation-dir and -fprofile-compilation-dir separately. This
is similar to -ffile-prefix-map.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97433

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CodeGen/debug-info-compilation-dir.c
  clang/test/CodeGen/profile-compilation-dir.c
  clang/test/Driver/clang_f_opts.c
  clang/tools/driver/cc1as_main.cpp

Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -241,7 +241,8 @@
   Opts.DwarfDebugProducer =
   std::string(Args.getLastArgValue(OPT_dwarf_debug_producer));
   Opts.DebugCompilationDir =
-  std::string(Args.getLastArgValue(OPT_fdebug_compilation_dir));
+  std::string(Args.getLastArgValue(OPT_ffile_compilation_dir,
+   OPT_fdebug_compilation_dir));
   Opts.MainFileName = std::string(Args.getLastArgValue(OPT_main_file_name));
 
   for (const auto  : Args.getAllArgValues(OPT_fdebug_prefix_map_EQ)) {
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -504,6 +504,8 @@
 // RUN: %clang -### -S -fdebug-compilation-dir=. %s 2>&1 | FileCheck -check-prefix=CHECK-DEBUG-COMPILATION-DIR %s
 // RUN: %clang -### -fdebug-compilation-dir . -x assembler %s 2>&1 | FileCheck -check-prefix=CHECK-DEBUG-COMPILATION-DIR %s
 // RUN: %clang -### -fdebug-compilation-dir=. -x assembler %s 2>&1 | FileCheck -check-prefix=CHECK-DEBUG-COMPILATION-DIR %s
+// RUN: %clang -### -S -ffile-compilation-dir=. %s 2>&1 | FileCheck -check-prefix=CHECK-DEBUG-COMPILATION-DIR %s
+// RUN: %clang -### -ffile-compilation-dir=. -x assembler %s 2>&1 | FileCheck -check-prefix=CHECK-DEBUG-COMPILATION-DIR %s
 // CHECK-DEBUG-COMPILATION-DIR: "-fdebug-compilation-dir=."
 
 // RUN: %clang -### -S -fdiscard-value-names %s 2>&1 | FileCheck -check-prefix=CHECK-DISCARD-NAMES %s
Index: clang/test/CodeGen/profile-compilation-dir.c
===
--- clang/test/CodeGen/profile-compilation-dir.c
+++ clang/test/CodeGen/profile-compilation-dir.c
@@ -1,6 +1,7 @@
 // RUN: mkdir -p %t.dir && cd %t.dir
 // RUN: cp %s rel.c
 // RUN: %clang_cc1 -fprofile-instrument=clang -fprofile-compilation-dir=/nonsense -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false rel.c -o - | FileCheck -check-prefix=CHECK-NONSENSE %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -ffile-compilation-dir=/nonsense -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false rel.c -o - | FileCheck -check-prefix=CHECK-NONSENSE %s
 
 // CHECK-NONSENSE: nonsense
 
Index: clang/test/CodeGen/debug-info-compilation-dir.c
===
--- clang/test/CodeGen/debug-info-compilation-dir.c
+++ clang/test/CodeGen/debug-info-compilation-dir.c
@@ -2,6 +2,7 @@
 // RUN: cp %s rel.c
 // RUN: %clang_cc1 -fdebug-compilation-dir /nonsense -emit-llvm -debug-info-kind=limited rel.c -o - | FileCheck -check-prefix=CHECK-NONSENSE %s
 // RUN: %clang_cc1 -fdebug-compilation-dir=/nonsense -emit-llvm -debug-info-kind=limited rel.c -o - | FileCheck -check-prefix=CHECK-NONSENSE %s
+// RUN: %clang_cc1 -ffile-compilation-dir=/nonsense -emit-llvm -debug-info-kind=limited rel.c -o - | FileCheck -check-prefix=CHECK-NONSENSE %s
 // CHECK-NONSENSE: nonsense
 
 // RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck -check-prefix=CHECK-DIR %s
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1011,7 +1011,8 @@
   } else {
 // Use the compilation dir.
 SmallString<128> T(
-Args.getLastArgValue(options::OPT_fdebug_compilation_dir_EQ));
+Args.getLastArgValue(options::OPT_ffile_compilation_dir_EQ,
+ options::OPT_fdebug_compilation_dir_EQ));
 SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput()));
 AddPostfix(F);
 T += F;
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -618,7 +618,8 @@
 /// Add a CC1 option to specify the debug compilation directory.
 static void addDebugCompDirArg(const ArgList , ArgStringList ,
   

[PATCH] D90448: [clang] Add type check for explicit instantiation of static data members

[Chuyang Chen via Phabricator via cfe-commits]

nomanous added a comment.

Ping @rsmith @dblaikie @MaskRay @haowei @Xiangling_L @lebedev.ri


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90448

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

[PATCH] D97340: [HIP] Support Spack packages

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 326254.
yaxunl marked 3 inline comments as done.
yaxunl added a comment.

revised by Artem's comments


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

https://reviews.llvm.org/D97340

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/ROCm.h
  
clang/test/Driver/Inputs/rocm-spack/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/bin/.hipVersion
  
clang/test/Driver/Inputs/rocm-spack/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/include/hip/hip_runtime.h
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/asanrtl.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/hip.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/ockl.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_correctly_rounded_sqrt_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_correctly_rounded_sqrt_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_daz_opt_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_daz_opt_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_finite_only_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_finite_only_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_1010.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_1011.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_1012.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_803.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_900.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_908.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_unsafe_math_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_unsafe_math_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_wavefrontsize64_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_wavefrontsize64_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/ocml.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/opencl.bc
  clang/test/Driver/rocm-detect.hip

Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -16,14 +16,71 @@
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,GFX902-DEFAULTLIBS %s
 
-
 // RUN: %clang -### -v -target x86_64-linux-gnu --cuda-gpu-arch=gfx902 -nogpulib \
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,NODEFAULTLIBS %s
 
+// Test ROCm installation built by SPACK by invoke clang at %T/rocm-spack/llvm-amdgpu-*
+// directory through a soft link.
+
+// RUN: rm -rf %T/rocm-spack
+// RUN: cp -r %S/Inputs/rocm-spack %T
+// RUN: ln -fs %clang %T/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang
+// RUN: %T/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -v \
+// RUN:   -target x86_64-linux-gnu --cuda-gpu-arch=gfx900 %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=SPACK %s
+
+// Test SPACK installation with multiple hip and rocm-device-libs packages of the same
+// ROCm release. Clang cannot determine which one to use and emit diagnostics. --hip-path
+// and --rocm-device-lib-path can be used to specify them.
+
+// RUN: cp -r %T/rocm-spack/hip-* %T/rocm-spack/hip-4.0.0-abcd
+// RUN: cp -r %T/rocm-spack/rocm-device-libs-* %T/rocm-spack/rocm-device-libs-4.0.0-efgh
+// RUN: %T/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -v \
+// RUN:   -target x86_64-linux-gnu --cuda-gpu-arch=gfx900 %s 2>&1 \
+// RUN:   | FileCheck 

[PATCH] D97340: [HIP] Support Spack packages

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 3 inline comments as done.
yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:25-31
+// Look for sub-directory starts with Prefix under Path. If there is one and
+// only one matching sub-directory found, append the sub-directory to Path. If
+// there is no matching sub-directory or there are more than one matching
+// sub-directories, diagnose them. Returns true if there is only one matching
+// sub-directory.
+static void handleSPACKPackage(const Driver , SmallString<0> ,
+   StringRef Prefix) {

tra wrote:
> `void` function can not return `true`, so the comment needs updating.
> 
> Also, Using Path as both an input and an output is odd. We should probably 
> return the full path or an empty string and call the function 
> `findSPACKPackage`.
done



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:27-28
+// only one matching sub-directory found, append the sub-directory to Path. If
+// there is no matching sub-directory or there are more than one matching
+// sub-directories, diagnose them. Returns true if there is only one matching
+// sub-directory.

tra wrote:
> How are users expected to handle cases when they do have multiple rocm 
> versions installed with spack (I assume that having multiple packages  *is* 
> possible. E.g. package-4.0, package-3.7 should be able to co-exist). They may 
> have legitimate reasons to have more than one rocm version installed *and* 
> they may need to be able to build with a particular one.
> 
> Can users use `--hip-path=specific/hip/version/in-spack` ?
Yes. --hip-path is introduced for that purpose.



Comment at: clang/lib/Driver/ToolChains/ROCm.h:56
+// convention --.
+llvm::SmallString<0> SPACKReleaseStr;
+  };

tra wrote:
> Nit: using `SmallString` here and everywhere else in this patch does not buy 
> us anything. `std::string` would be fine.
> My own rule of thumb is to use standard types by default and optimized types 
> when they are needed. 
thanks. will use std::string


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

https://reviews.llvm.org/D97340

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

[PATCH] D97358: [X86] Support amx-bf16 intrinsic.

[LiuChen 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 rG4bc7c8631ad6: [X86] Support amx-bf16 intrinsic. (authored by 
LiuChen3).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97358

Files:
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/lib/Headers/amxintrin.h
  clang/test/CodeGen/X86/amx_api.c
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/Target/X86/X86ExpandPseudo.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86InstrAMX.td
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86PreTileConfig.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll

Index: llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll
===
--- llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll
+++ llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+amx-tile -mattr=+avx512f -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+amx-tile,+amx-int8,+amx-bf16,+avx512f -verify-machineinstrs | FileCheck %s
 
 define void @test_amx(i8* %pointer, i8* %base, i64 %stride) {
 ; CHECK-LABEL: test_amx:
@@ -22,6 +22,7 @@
 ; CHECK-NEXT:tdpbsud %tmm2, %tmm1, %tmm0
 ; CHECK-NEXT:tdpbusd %tmm2, %tmm1, %tmm0
 ; CHECK-NEXT:tdpbuud %tmm2, %tmm1, %tmm0
+; CHECK-NEXT:tdpbf16ps %tmm2, %tmm1, %tmm0
 ; CHECK-NEXT:tilestored %tmm0, (%rdi,%rdx)
 ; CHECK-NEXT:tilerelease
 ; CHECK-NEXT:vzeroupper
@@ -33,7 +34,8 @@
   %d1 = call x86_amx @llvm.x86.tdpbsud.internal(i16 8, i16 8, i16 8, x86_amx %d0, x86_amx %a, x86_amx %b)
   %d2 = call x86_amx @llvm.x86.tdpbusd.internal(i16 8, i16 8, i16 8, x86_amx %d1, x86_amx %a, x86_amx %b)
   %d3 = call x86_amx @llvm.x86.tdpbuud.internal(i16 8, i16 8, i16 8, x86_amx %d2, x86_amx %a, x86_amx %b)
-  call void @llvm.x86.tilestored64.internal(i16 8, i16 8, i8* %pointer, i64 %stride, x86_amx %d3)
+  %d4 = call x86_amx @llvm.x86.tdpbf16ps.internal(i16 8, i16 8, i16 8, x86_amx %d3, x86_amx %a, x86_amx %b)
+  call void @llvm.x86.tilestored64.internal(i16 8, i16 8, i8* %pointer, i64 %stride, x86_amx %d4)
 
   ret void
 }
@@ -44,4 +46,5 @@
 declare x86_amx @llvm.x86.tdpbsud.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
 declare x86_amx @llvm.x86.tdpbusd.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
 declare x86_amx @llvm.x86.tdpbuud.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
+declare x86_amx @llvm.x86.tdpbf16ps.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
 declare void @llvm.x86.tilestored64.internal(i16, i16, i8*, i64, x86_amx)
Index: llvm/lib/Target/X86/X86RegisterInfo.cpp
===
--- llvm/lib/Target/X86/X86RegisterInfo.cpp
+++ llvm/lib/Target/X86/X86RegisterInfo.cpp
@@ -888,6 +888,7 @@
   case X86::PTDPBUSDV:
   case X86::PTDPBUUDV:
   case X86::PTILEZEROV:
+  case X86::PTDPBF16PSV:
 MachineOperand  = MI->getOperand(1);
 MachineOperand  = MI->getOperand(2);
 ShapeT Shape(, , MRI);
Index: llvm/lib/Target/X86/X86PreTileConfig.cpp
===
--- llvm/lib/Target/X86/X86PreTileConfig.cpp
+++ llvm/lib/Target/X86/X86PreTileConfig.cpp
@@ -159,6 +159,7 @@
   case X86::PTDPBUSDV:
   case X86::PTDPBUUDV:
   case X86::PTILEZEROV:
+  case X86::PTDPBF16PSV:
 MachineOperand  = const_cast(MI.getOperand(1));
 MachineOperand  = const_cast(MI.getOperand(2));
 ShapeT Shape(, , MRI);
@@ -256,6 +257,7 @@
   case X86::PTDPBUSDV:
   case X86::PTDPBUUDV:
   case X86::PTILEZEROV:
+  case X86::PTDPBF16PSV:
 return true;
   }
 }
Index: llvm/lib/Target/X86/X86LowerAMXType.cpp
===
--- llvm/lib/Target/X86/X86LowerAMXType.cpp
+++ llvm/lib/Target/X86/X86LowerAMXType.cpp
@@ -70,7 +70,8 @@
   case Intrinsic::x86_tdpbssd_internal:
   case Intrinsic::x86_tdpbsud_internal:
   case Intrinsic::x86_tdpbusd_internal:
-  case Intrinsic::x86_tdpbuud_internal: {
+  case Intrinsic::x86_tdpbuud_internal:
+  case Intrinsic::x86_tdpbf16ps_internal: {
 switch (OpNo) {
 case 3:
   Row = II->getArgOperand(0);
Index: llvm/lib/Target/X86/X86InstrAMX.td
===
--- llvm/lib/Target/X86/X86InstrAMX.td
+++ llvm/lib/Target/X86/X86InstrAMX.td
@@ -138,6 +138,16 @@
   "tdpbf16ps\t{$src3, $src2, $dst|$dst, $src2, $src3}",
   []>, VEX_4V, T8XS;
 
+// Pseduo instruction for RA.
+let Constraints = "$src4 = $dst" in
+  def PTDPBF16PSV : PseudoI<(outs TILE: $dst), (ins GR16:$src1,
+ GR16:$src2, GR16:$src3, 

[clang] 4bc7c86 - [X86] Support amx-bf16 intrinsic.

[via cfe-commits]

Author: Liu, Chen3
Date: 2021-02-25T09:06:48+08:00
New Revision: 4bc7c8631ad62487a290dd4b7791848b67635787

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

LOG: [X86] Support amx-bf16 intrinsic.

Adding support for intrinsics of AMX-BF16.
This patch alse fix a bug that AMX-INT8 instructions will be selected with wrong
predicate.

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsX86_64.def
clang/lib/Headers/amxintrin.h
clang/test/CodeGen/X86/amx_api.c
llvm/include/llvm/IR/IntrinsicsX86.td
llvm/lib/Target/X86/X86ExpandPseudo.cpp
llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
llvm/lib/Target/X86/X86InstrAMX.td
llvm/lib/Target/X86/X86LowerAMXType.cpp
llvm/lib/Target/X86/X86PreTileConfig.cpp
llvm/lib/Target/X86/X86RegisterInfo.cpp
llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsX86_64.def 
b/clang/include/clang/Basic/BuiltinsX86_64.def
index aed46b352342..731f17452cbe 100644
--- a/clang/include/clang/Basic/BuiltinsX86_64.def
+++ b/clang/include/clang/Basic/BuiltinsX86_64.def
@@ -108,6 +108,7 @@ TARGET_BUILTIN(__builtin_ia32_tdpbusd_internal, 
"V256iUsUsUsV256iV256iV256i", "n
 TARGET_BUILTIN(__builtin_ia32_tdpbuud_internal, "V256iUsUsUsV256iV256iV256i", 
"n", "amx-int8")
 TARGET_BUILTIN(__builtin_ia32_tilestored64_internal, "vUsUsv*zV256i", "n", 
"amx-tile")
 TARGET_BUILTIN(__builtin_ia32_tilezero_internal, "V256iUsUs", "n", "amx-tile")
+TARGET_BUILTIN(__builtin_ia32_tdpbf16ps_internal, 
"V256iUsUsUsV256iV256iV256i", "n", "amx-bf16")
 // AMX
 TARGET_BUILTIN(__builtin_ia32_tile_loadconfig, "vvC*", "n", "amx-tile")
 TARGET_BUILTIN(__builtin_ia32_tile_storeconfig, "vvC*", "n", "amx-tile")

diff  --git a/clang/lib/Headers/amxintrin.h b/clang/lib/Headers/amxintrin.h
index 31a2b64b9ff2..8c276519e362 100644
--- a/clang/lib/Headers/amxintrin.h
+++ b/clang/lib/Headers/amxintrin.h
@@ -15,8 +15,13 @@
 #define __AMXINTRIN_H
 #ifdef __x86_64__
 
+/* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS_TILE
\
   __attribute__((__always_inline__, __nodebug__, __target__("amx-tile")))
+#define __DEFAULT_FN_ATTRS_INT8
\
+  __attribute__((__always_inline__, __nodebug__, __target__("amx-int8")))
+#define __DEFAULT_FN_ATTRS_BF16
\
+  __attribute__((__always_inline__, __nodebug__, __target__("amx-bf16")))
 
 /// Load tile configuration from a 64-byte memory location specified by
 /// "mem_addr". The tile configuration includes the tile type palette, the
@@ -221,10 +226,8 @@ static __inline__ void __DEFAULT_FN_ATTRS_TILE 
_tile_release(void) {
 #define _tile_dpbf16ps(dst, src0, src1)
\
   __builtin_ia32_tdpbf16ps((dst), (src0), (src1))
 
-#define __DEFAULT_FN_ATTRS_INT8
\
-  __attribute__((__always_inline__, __nodebug__, __target__("amx-int8")))
-
 typedef int _tile1024i __attribute__((__vector_size__(1024), __aligned__(64)));
+
 static __inline__ _tile1024i __DEFAULT_FN_ATTRS_INT8
 _tile_loadd_internal(unsigned short m, unsigned short n, const void *base,
  __SIZE_TYPE__ stride) {
@@ -263,6 +266,12 @@ _tile_stored_internal(unsigned short m, unsigned short n, 
void *base,
   (__SIZE_TYPE__)(stride), tile);
 }
 
+static __inline__ _tile1024i __DEFAULT_FN_ATTRS_BF16
+_tile_tdpbf16ps_internal(unsigned short m, unsigned short n, unsigned short k,
+ _tile1024i dst, _tile1024i src1, _tile1024i src2) {
+  return __builtin_ia32_tdpbf16ps_internal(m, n, k, dst, src1, src2);
+}
+
 typedef struct __tile1024i_str {
   const unsigned short row;
   const unsigned short col;
@@ -313,5 +322,16 @@ static void __tile_zero(__tile1024i *dst) {
   dst->tile = __builtin_ia32_tilezero_internal(dst->row, dst->col);
 }
 
+__DEFAULT_FN_ATTRS_BF16
+static void __tile_tdpbf16ps(__tile1024i *dst, __tile1024i src1,
+ __tile1024i src2) {
+  dst->tile = _tile_tdpbf16ps_internal(src1.row, src2.col, src1.col, dst->tile,
+   src1.tile, src2.tile);
+}
+
+#undef __DEFAULT_FN_ATTRS_TILE
+#undef __DEFAULT_FN_ATTRS_INT8
+#undef __DEFAULT_FN_ATTRS_BF16
+
 #endif /* __x86_64__ */
 #endif /* __AMXINTRIN_H */

diff  --git a/clang/test/CodeGen/X86/amx_api.c 
b/clang/test/CodeGen/X86/amx_api.c
index 7120de4c9e88..824a3aec20ec 100644
--- a/clang/test/CodeGen/X86/amx_api.c
+++ b/clang/test/CodeGen/X86/amx_api.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -ffreestanding 

[PATCH] D97386: update AMDGPU _Float16 support in clang doc

[Yaxun Liu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG392fd3f1bf9f: update AMDGPU _Float16 support in clang doc 
(authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97386

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -524,6 +524,7 @@
 
 * 32-bit ARM
 * 64-bit ARM (AArch64)
+* AMDGPU
 * SPIR
 
 ``_Float16`` will be supported on more targets as they define ABIs for it.


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -524,6 +524,7 @@
 
 * 32-bit ARM
 * 64-bit ARM (AArch64)
+* AMDGPU
 * SPIR
 
 ``_Float16`` will be supported on more targets as they define ABIs for it.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 392fd3f - update AMDGPU _Float16 support in clang doc

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-02-24T19:46:23-05:00
New Revision: 392fd3f1bf9f925942b385d8b99fb662d5739a83

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

LOG: update AMDGPU _Float16 support in clang doc

Reviewed by: Matt Arsenault

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

Added: 


Modified: 
clang/docs/LanguageExtensions.rst

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 6fa6c55b15fc..4b82b8a6dbbc 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -524,6 +524,7 @@ targets pending ABI standardization:
 
 * 32-bit ARM
 * 64-bit ARM (AArch64)
+* AMDGPU
 * SPIR
 
 ``_Float16`` will be supported on more targets as they define ABIs for it.



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

[PATCH] D97204: [RFC] Clang 64-bit source locations

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

akyrtzi added a comment.

In D97204#2586111 , @rsmith wrote:

> Can we avoid a libclang ABI break if we don't allow the use of 64-bit source 
> locations for builds with 32-bit pointers?

To @rsmith's point, the simplest option may be to avoid building libclang if 
64-bit source locations are enabled.


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

https://reviews.llvm.org/D97204

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

[clang] 7c926fe - Improve attribute documentation for nodebug on typedefs

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2021-02-24T16:25:37-08:00
New Revision: 7c926fee930012f9ec19cdaab23b7e154a3845ba

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

LOG: Improve attribute documentation for nodebug on typedefs

(followup to 8472fa6c54c9d044adcd147f6826bccebd730f30 )

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 9de204956190..d7d74bd6eee7 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -1048,8 +1048,8 @@ def NoDebugDocs : Documentation {
   let Category = DocCatVariable;
   let Content = [{
 The ``nodebug`` attribute allows you to suppress debugging information for a
-function or method, or for a variable that is not a parameter or a non-static
-data member.
+function or method, for a variable that is not a parameter or a non-static
+data member, or for a typedef or using declaration.
   }];
 }
 



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

[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 updated this revision to Diff 326234.
arnamoy10 edited the summary of this revision.
arnamoy10 added a comment.

Addressing comments, by separating the search directories from 
`fintrinsic-modules-path` in a separate variables.  Also added dummy modules 
for the test case, as using `%llvmshlibdir` did not work out.

Also, not sure what to set as the default directory, as gfortran uses a 
specific installation location.


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

https://reviews.llvm.org/D97080

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/PreprocessorOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Flang-Driver/Inputs/ieee_arithmetic.mod
  flang/test/Flang-Driver/Inputs/iso_fortran_env.mod
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/intrinsic_module_path.f90

Index: flang/test/Flang-Driver/intrinsic_module_path.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/intrinsic_module_path.f90
@@ -0,0 +1,32 @@
+! Ensure argument -fintrinsic-modules-path works as expected.
+
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: not %flang-new -fsyntax-only %s  2>&1 | FileCheck %s --check-prefix=WITHOUT
+! RUN: not %flang-new -fsyntax-only -fintrinsic-modules-path %S/Inputs/ %s  2>&1 | FileCheck %s --check-prefix=GIVEN
+
+!-
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: not %flang-new -fc1 %s  2>&1 | FileCheck %s --check-prefix=WITHOUT
+! RUN: not %flang-new -fc1 -fintrinsic-modules-path %S/Inputs/ %s  2>&1 | FileCheck %s --check-prefix=GIVEN
+
+!-
+! EXPECTED OUTPUT WITHOUT
+!-
+! WITHOUT: 'ieee_arithmetic.mod' was not found
+! WITHOUT: 'iso_fortran_env.mod' was not found
+
+!-
+! EXPECTED OUTPUT WITH
+!-
+! GIVEN-NOT: 'ieee_arithmetic.mod' was not found
+! GIVEN-NOT: 'iso_fortran_env.mod' was not found
+
+
+program test_intrinsic_module_path
+   use ieee_arithmetic, only: ieee_round_type
+   use iso_fortran_env, only: team_type, event_type, lock_type
+end program
Index: flang/test/Flang-Driver/driver-help.f90
===
--- flang/test/Flang-Driver/driver-help.f90
+++ flang/test/Flang-Driver/driver-help.f90
@@ -32,6 +32,8 @@
 ! HELP-NEXT: -ffree-formProcess source files in free form
 ! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements
 ! HELP-NEXT: -finput-charset= Specify the default character set for source files
+! HELP-NEXT: -fintrinsic-modules-path 
+! HELP-NEXT:Specify where to find the compiled intrinsic modules.
 ! HELP-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
 ! HELP-NEXT: -fopenacc  Enable OpenACC
@@ -72,6 +74,8 @@
 ! HELP-FC1-NEXT: -ffree-formProcess source files in free form
 ! HELP-FC1-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements
 ! HELP-FC1-NEXT: -finput-charset= Specify the default character set for source files
+! HELP-FC1-NEXT: -fintrinsic-modules-path 
+! HELP-FC1-NEXT:Specify where to find the compiled intrinsic modules.
 ! HELP-FC1-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-FC1-NEXT: -fopenacc  Enable OpenACC
 ! HELP-FC1-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
Index: flang/test/Flang-Driver/driver-help-hidden.f90
===
--- flang/test/Flang-Driver/driver-help-hidden.f90
+++ flang/test/Flang-Driver/driver-help-hidden.f90
@@ -32,6 +32,8 @@
 ! CHECK-NEXT: -ffree-formProcess source files in free form
 ! CHECK-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements
 ! CHECK-NEXT: -finput-charset= Specify the default character set for source files
+! CHECK-NEXT: -fintrinsic-modules-path 
+! CHECK-NEXT:Specify where to find the compiled intrinsic modules.
 ! CHECK-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! CHECK-NEXT: -fno-color-diagnostics Disable colors in diagnostics
 ! CHECK-NEXT: -fopenacc  Enable OpenACC
Index: flang/test/Flang-Driver/Inputs/iso_fortran_env.mod
===
--- /dev/null
+++ flang/test/Flang-Driver/Inputs/iso_fortran_env.mod
@@ -0,0 +1,6 @@
+! DUMMY module
+module iso_fortran_env
+use 

[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:8326-8331
+template 
+StmtResult
+TreeTransform::TransformOMPCanonicalLoop(OMPCanonicalLoop *L) {
+  llvm_unreachable("OMPCanonicalLoop must be handled by the "
+   "OMPExecutableDirective that it is associated with");
+}

Because this receives an OMPCanonicalLoop, I'm assuming the OpenMP IR builder 
is enabled.  Without my suggested edits, the caller checks that.  After my 
suggested edits, maybe it's worthwhile to have an assertion here that it's 
enabled.  I don't know.



Comment at: clang/lib/Sema/TreeTransform.h:8368-8374
+  if (isOpenMPLoopDirective(D->getDirectiveKind()) &&
+  getSema().getLangOpts().OpenMPIRBuilder)
+CS = cast(CS)->getLoopStmt();
   Body = getDerived().TransformStmt(CS);
+  if (Body.isUsable() && isOpenMPLoopDirective(D->getDirectiveKind()) &&
+  getSema().getLangOpts().OpenMPIRBuilder)
+Body = getDerived().RebuildOMPCanonicalLoop(Body.get());





Comment at: clang/lib/Sema/TreeTransform.h:8321
+TreeTransform::TransformOMPCanonicalLoop(OMPCanonicalLoop *L) {
+  // The OMPCanonicalLoop will be recreated when transforming the 
loop-associted
+  // directive.

Meinersbur wrote:
> jdenny wrote:
> > Meinersbur wrote:
> > > jdenny wrote:
> > > > I'm used to seeing `TransformX` call `RebuildX` call `ActOnX`.  Why not 
> > > > do that for `X=OMPCanonicalLoop`?  Does 
> > > > `TransformOMPExecutableDirective` really need a special case for 
> > > > `OMPCanonicalLoop`?
> > > The intended behaviour is: 
> > > 
> > > 1. Transform the child loop
> > > 2. Return the child loop as representing the OMPCanonicalLoop (i.e. 
> > > OMPCanonicalLoop wrapper is removed).
> > > 3. Parent loop-associated directive (e.g. workshare) is processed.
> > > 4. `ActOnOpenMPCanonicalLoop` adds new OMPCanonicalLoop wrapper for loop 
> > > nest.
> > > 
> > > This guarantees maximum flexibility on what of the loop can be changed, 
> > > such as
> > > * Change lower bound, upper bound, step
> > > * Convert between CXXForRangeStmt and ForStmt
> > > * Change the associated depth (e.g. different value for `collapse` clause)
> > > * Remove the directive and no OMPCanonicalLoop remain
> > > 
> > > This also saves adding many lines of code handling transforming each 
> > > member of OMPCanonicalLoop separately.
> > > The intended behaviour is: 
> > > 
> > > 1. Transform the child loop
> > 
> > For  my suggestion, that call would remain within 
> > `TransformOMPCanonicalLoop` where it is now.
> > 
> > > 2. Return the child loop as representing the OMPCanonicalLoop (i.e. 
> > > OMPCanonicalLoop wrapper is removed).
> > 
> > For my suggestion, this would not happen.  I think it's normal for a 
> > `TransformX` to return the transformed `X` not the transformed child of 
> > `X`.  If a caller wants to transform the child, then it should transform 
> > the child directly instead.
> > 
> > > 3. Parent loop-associated directive (e.g. workshare) is processed.
> > 
> > It looks to me like step 3 is currently within 
> > `TransformOMPExecutableDirective` and starts before the call to 
> > `TranformOMPCanonicalLoop` and thus before step 1.  It completes after step 
> > 4.  Or am I misunderstanding what you're describing as step 3?
> > 
> > > 4. `ActOnOpenMPCanonicalLoop` adds new OMPCanonicalLoop wrapper for loop 
> > > nest.
> > 
> > For my suggestion, this would still happen.  However, instead of step 2: 
> > within `TransformOMPCanonicalLoop`, you would call 
> > `RebuildOMPCanonicalLoop`, which would call `ActOnOpenMPCanonicalLoop` as 
> > step 4.  The effect is you moved `ActOnOpenMPCanonicalLoop` from the caller 
> > (`TransformOMPExecutableDirective`) to the callee's callee, but the 
> > behavior should remain the same.
> > 
> > > This guarantees maximum flexibility on what of the loop can be changed, 
> > > such as
> > > * Change lower bound, upper bound, step
> > > * Convert between CXXForRangeStmt and ForStmt
> > > * Change the associated depth (e.g. different value for `collapse` clause)
> > > * Remove the directive and no OMPCanonicalLoop remain
> > 
> > Flexibility for whom?
> > 
> > A class extending `TreeTransform`?  With my suggestion, it can override 
> > `TransformOMPCanonicalLoop` or `RebuildOMPCanonicalLoop`, depending on how 
> > much it wants to alter the transformation.
> > 
> > Or a caller of `TransformOMPCanonicalLoop` within `TreeTransform`?  I only 
> > see one right now, `TransformOMPExecutableDirective`, and I don't see how 
> > it needs the flexibility.  Are there other callers I missed?
> > 
> > Are you trying to create flexibility without requiring deriving from 
> > `TreeTransform`?  But, as far as I can tell, you're doing so at the expense 
> > of normal `TreeTransform` semantics.  Doesn't seem worth it.
> > 
> > If you see a precedent for your approach elsewhere in 

[PATCH] D96835: [HIP] Support device sanitizer

[Albion Fung via Phabricator via cfe-commits]

Conanap added a comment.

Hello,

One of our PowerPC buildbots is failing because it is named `lld-multistage`, 
which matches with this CHECK-NOT in 
`clang/test/Driver/hip-sanitize-options.hip`: `;CHECK-NOT: {{"[^"]*lld[^"]*".* 
".*hip.bc"}}`.
I've created a patch and if you could review it that'd be great!

https://reviews.llvm.org/D97423

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96835

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

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D97411#2586055 , @rnk wrote:

> I expected nodebug already applied to types, even though the documentation 
> says it only affects variables and functions. We should update the docs.

Now that I think about it more, the only thing I made it work for is typedefs, 
not whole structs - it'd be trickier to do it for a whole struct (what happens 
if you have a member of that type in another struct? Does it leave a black 
hole?) - but easy for a typedef: any reference to the typedef is instead a 
direct reference to the underlying type.

> I think if we already have `nodebug` spelling, we don't need to make 
> something general with modes. The "always emit" use case is fairly weak. It's 
> easy to drop in a `static_assert(sizeof(Ty) > 0, "emit type info");` to get 
> type info if you want it.

I don't think that static_assert would cause 'Ty' to be emitted - at least for 
DWARF/ELF I don't know of a way to force a type to be used that doesn't 
generate any code, for instance.

> If we're looking at a no-argument attribute, my naming ideas lean towards 
> incorporating "required [to be] complete" in the name. There are many ways 
> that one can "use" or "require" a type that work fine with a forward 
> declaration. Something like `emit_debug_typeinfo_if_required_complete`? Still 
> too big? `emit_debug_if_required_complete`?

Oh, that might be a bit different again - if the type isn't required to be 
complete in this translation unit, should this attribute override the "required 
to be complete" homing strategy? (that's the oldest standing strategy - if the 
type isn't required to be complete, the definition is omitted) I'd have thought 
this should override that behavior too. Perhaps the type is never dereferenced, 
but is somehow still useful (eg: you might have one translation unit that only 
uses handles, and another translation unit that never has a variable of that 
type (maybe deals with void*) and casts to the defined type and dereferences it 
- that would break the "required to be complete" homing strategy (though it's 
unlikely/weird - if you're passing around void* it's unlikely your caller would 
somehow see and use the declared type anyway))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411

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

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

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
yaxunl marked 6 inline comments as done.
Closed by commit rG47acdec1dd5d: [CUDA][HIP] Support accessing static device 
variable in host code for -fgpu-rdc (authored by yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D85223?vs=322021=326223#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85223

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCUDA/device-var-linkage.cu
  clang/test/CodeGenCUDA/managed-var.cu
  clang/test/CodeGenCUDA/static-device-var-rdc.cu
  clang/test/SemaCUDA/static-device-var.cu

Index: clang/test/SemaCUDA/static-device-var.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/static-device-var.cu
@@ -0,0 +1,50 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple nvptx -fcuda-is-device \
+// RUN:-emit-llvm -o - %s -fsyntax-only -verify=dev
+
+// RUN: %clang_cc1 -triple x86_64-gnu-linux \
+// RUN:-emit-llvm -o - %s -fsyntax-only -verify=host
+
+// Checks allowed usage of file-scope and function-scope static variables.
+
+// host-no-diagnostics
+
+#include "Inputs/cuda.h"
+
+// Checks static variables are allowed in device functions.
+
+__device__ void f1() {
+  const static int b = 123;
+  static int a;
+}
+
+// Checks static variables are allowd in global functions.
+
+__global__ void k1() {
+  const static int b = 123;
+  static int a;
+}
+
+// Checks static device and constant variables are allowed in device and
+// host functions, and static host variables are not allowed in device
+// functions.
+
+static __device__ int x;
+static __constant__ int y;
+static int z;
+
+__global__ void kernel(int *a) {
+  a[0] = x;
+  a[1] = y;
+  a[2] = z;
+  // dev-error@-1 {{reference to __host__ variable 'z' in __global__ function}}
+}
+
+int* getDeviceSymbol(int *x);
+
+void foo() {
+  getDeviceSymbol();
+  getDeviceSymbol();
+}
Index: clang/test/CodeGenCUDA/static-device-var-rdc.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/static-device-var-rdc.cu
@@ -0,0 +1,97 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN:   -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
+// RUN:   -check-prefixes=DEV,INT-DEV %s
+
+// RUN: %clang_cc1 -triple x86_64-gnu-linux \
+// RUN:   -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
+// RUN:   -check-prefixes=HOST,INT-HOST %s
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -cuid=abc \
+// RUN:   -fgpu-rdc -emit-llvm -o - -x hip %s > %t.dev
+// RUN: cat %t.dev | FileCheck -check-prefixes=DEV,EXT-DEV %s
+
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -cuid=abc \
+// RUN:   -fgpu-rdc -emit-llvm -o - -x hip %s > %t.host
+// RUN: cat %t.host | FileCheck -check-prefixes=HOST,EXT-HOST %s
+
+// Check host and device compilations use the same postfixes for static
+// variable names.
+
+// RUN: cat %t.dev %t.host | FileCheck -check-prefix=POSTFIX %s
+
+#include "Inputs/cuda.h"
+
+// Test function scope static device variable, which should not be externalized.
+// DEV-DAG: @_ZZ6kernelPiPPKiE1w = internal addrspace(4) constant i32 1
+
+
+// HOST-DAG: @_ZL1x = internal global i32 undef
+// HOST-DAG: @_ZL1y = internal global i32 undef
+
+// Test normal static device variables
+// INT-DEV-DAG: @_ZL1x = dso_local addrspace(1) externally_initialized global i32 0
+// INT-HOST-DAG: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x\00"
+
+// Test externalized static device variables
+// EXT-DEV-DAG: @_ZL1x.static.[[HASH:.*]] = dso_local addrspace(1) externally_initialized global i32 0
+// EXT-HOST-DAG: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x.static.[[HASH:.*]]\00"
+
+// POSTFIX: @_ZL1x.static.[[HASH:.*]] = dso_local addrspace(1) externally_initialized global i32 0
+// POSTFIX: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x.static.[[HASH]]\00"
+
+static __device__ int x;
+
+// Test static device variables not used by host code should not be externalized
+// DEV-DAG: @_ZL2x2 = internal addrspace(1) global i32 0
+
+static __device__ int x2;
+
+// Test normal static device variables
+// INT-DEV-DAG: @_ZL1y = dso_local addrspace(4) externally_initialized global i32 0
+// INT-HOST-DAG: @[[DEVNAMEY:[0-9]+]] = {{.*}}c"_ZL1y\00"
+
+// Test externalized static device variables
+// EXT-DEV-DAG: @_ZL1y.static.[[HASH]] = dso_local addrspace(4) externally_initialized global i32 0
+// EXT-HOST-DAG: @[[DEVNAMEY:[0-9]+]] = {{.*}}c"_ZL1y.static.[[HASH]]\00"
+
+static __constant__ int y;
+
+// Test static host variable, which should not be externalized nor registered.
+// 

[clang] 47acdec - [CUDA][HIP] Support accessing static device variable in host code for -fgpu-rdc

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-02-24T18:23:45-05:00
New Revision: 47acdec1dd5d6d4c279727a97313c586c20e9c6f

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

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

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

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

Reviewed by: Artem Belevich, and Jon Chesterfield

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

Added: 
clang/test/CodeGenCUDA/static-device-var-rdc.cu
clang/test/SemaCUDA/static-device-var.cu

Modified: 
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/CodeGen/CGCUDANV.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCUDA/device-var-linkage.cu
clang/test/CodeGenCUDA/managed-var.cu

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 8ba4c41aac701..af2979d874382 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -299,6 +299,10 @@ class ASTContext : public RefCountedBase {
   /// This is lazily created.  This is intentionally not serialized.
   mutable llvm::StringMap StringLiteralCache;
 
+  /// MD5 hash of CUID. It is calculated when first used and cached by this
+  /// data member.
+  mutable std::string CUIDHash;
+
   /// Representation of a "canonical" template template parameter that
   /// is used in canonical template names.
   class CanonicalTemplateTemplateParm : public llvm::FoldingSetNode {
@@ -3117,6 +3121,8 @@ OPT_LIST(V)
   /// Whether a C++ static variable should be externalized.
   bool shouldExternalizeStaticVar(const Decl *D) const;
 
+  StringRef getCUIDHash() const;
+
 private:
   /// All OMPTraitInfo objects live in this collection, one per
   /// `pragma omp [begin] declare variant` directive.

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index fcbc3f6252b6e..b8231f66908a9 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -84,6 +84,7 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/MD5.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -10645,7 +10646,10 @@ static GVALinkage adjustGVALinkageForAttributes(const 
ASTContext ,
   return GVA_StrongODR;
 // Single source offloading languages like CUDA/HIP need to be able to
 // access static device variables from host code of the same compilation
-// unit. This is done by externalizing the static variable.
+// unit. This is done by externalizing the static variable with a shared
+// name between the host and device compilation which is the same for the
+// same compilation unit whereas 
diff erent among 
diff erent compilation
+// units.
 if (Context.shouldExternalizeStaticVar(D))
   return GVA_StrongExternal;
   }
@@ -11533,10 +11537,8 @@ bool ASTContext::mayExternalizeStaticVar(const Decl 
*D) const {
   !D->getAttr()->isImplicit());
   // CUDA/HIP: static managed variables need to be externalized since it is
   // a declaration in IR, therefore cannot have internal linkage.
-  // ToDo: externalize static variables for -fgpu-rdc.
   return IsStaticVar &&
- (D->hasAttr() ||
-  (!getLangOpts().GPURelocatableDeviceCode && IsExplicitDeviceVar));
+ (D->hasAttr() || IsExplicitDeviceVar);
 }
 
 bool ASTContext::shouldExternalizeStaticVar(const Decl *D) const {
@@ -11544,3 +11546,12 @@ bool ASTContext::shouldExternalizeStaticVar(const Decl 
*D) const {
  (D->hasAttr() ||
   CUDAStaticDeviceVarReferencedByHost.count(cast(D)));
 }
+
+StringRef ASTContext::getCUIDHash() const {
+  if (!CUIDHash.empty())
+return CUIDHash;
+  if (LangOpts.CUID.empty())
+return StringRef();
+  CUIDHash = llvm::utohexstr(llvm::MD5Hash(LangOpts.CUID), /*LowerCase=*/true);
+  return CUIDHash;
+}

diff  --git a/clang/lib/CodeGen/CGCUDANV.cpp b/clang/lib/CodeGen/CGCUDANV.cpp
index dccbd6f74a981..57e3b151bfd42 100644
--- a/clang/lib/CodeGen/CGCUDANV.cpp
+++ b/clang/lib/CodeGen/CGCUDANV.cpp
@@ -255,6 +255,17 @@ std::string CGNVCUDARuntime::getDeviceSideName(const 
NamedDecl *ND) {
 DeviceSideName = 

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 326222.
zequanwu marked 2 inline comments as done.
zequanwu added a comment.

Add a doc example and CodeGen testcase about `not_tail_called` being applied to 
method override in a derived class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96832

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenCXX/attr-notail.cpp
  clang/test/SemaCXX/attr-notail.cpp

Index: clang/test/SemaCXX/attr-notail.cpp
===
--- clang/test/SemaCXX/attr-notail.cpp
+++ clang/test/SemaCXX/attr-notail.cpp
@@ -1,8 +1,9 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// expected-no-diagnostics
 
 class Base {
 public:
-  [[clang::not_tail_called]] virtual int foo1(); // expected-error {{'not_tail_called' attribute cannot be applied to virtual functions}}
+  [[clang::not_tail_called]] virtual int foo1();
   virtual int foo2();
   [[clang::not_tail_called]] int foo3();
   virtual ~Base() {}
@@ -11,6 +12,6 @@
 class Derived1 : public Base {
 public:
   int foo1() override;
-  [[clang::not_tail_called]] int foo2() override; // expected-error {{'not_tail_called' attribute cannot be applied to virtual functions}}
+  [[clang::not_tail_called]] int foo2() override;
   [[clang::not_tail_called]] int foo4();
 };
Index: clang/test/CodeGenCXX/attr-notail.cpp
===
--- clang/test/CodeGenCXX/attr-notail.cpp
+++ clang/test/CodeGenCXX/attr-notail.cpp
@@ -4,14 +4,28 @@
 public:
   [[clang::not_tail_called]] int m1();
   int m2();
+  [[clang::not_tail_called]] virtual int m3();
+  virtual int m4();
 };
 
-int foo1(int a, Class1 *c1) {
+class Class2: public Class1 {
+public:
+  [[clang::not_tail_called]] int m4() override;
+};
+
+int foo1(int a, Class1 *c1, Class2 ) {
   if (a)
 return c1->m1();
+  c1->m3();
+  Class1  = c2;
+  c.m4();
+  c2.m4();
   return c1->m2();
 }
 
-// CHECK-LABEL: define{{.*}} i32 @_Z4foo1iP6Class1(
+// CHECK-LABEL: define{{.*}} i32 @_Z4foo1iP6Class1R6Class2(
 // CHECK: %{{[a-z0-9]+}} = notail call i32 @_ZN6Class12m1Ev(%class.Class1*
+// CHECK: %{{[a-z0-9]+}} = notail call i32 %{{[0-9]+}}(%class.Class1*
+// CHECK-NOT: %{{[a-z0-9]+}} = notail call i32 %{{[0-9]+}}(%class.Class1*
+// CHECK: %{{[a-z0-9]+}} = notail call i32 %{{[0-9]+}}(%class.Class2* 
 // CHECK: %{{[a-z0-9]+}} = call i32 @_ZN6Class12m2Ev(%class.Class1*
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6432,16 +6432,6 @@
 }
   }
 
-  // Virtual functions cannot be marked as 'notail'.
-  if (auto *Attr = ND.getAttr())
-if (auto *MD = dyn_cast())
-  if (MD->isVirtual()) {
-S.Diag(ND.getLocation(),
-   diag::err_invalid_attribute_on_virtual_function)
-<< Attr;
-ND.dropAttr();
-  }
-
   // Check the attributes on the function type, if any.
   if (const auto *FD = dyn_cast()) {
 // Don't declare this variable in the second operand of the for-statement;
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4057,9 +4057,8 @@
   let Category = DocCatFunction;
   let Content = [{
 The ``not_tail_called`` attribute prevents tail-call optimization on statically
-bound calls. It has no effect on indirect calls. Virtual functions, objective-c
-methods, and functions marked as ``always_inline`` cannot be marked as
-``not_tail_called``.
+bound calls. Objective-c methods, and functions marked as ``always_inline``
+cannot be marked as ``not_tail_called``.
 
 For example, it prevents tail-call optimization in the following case:
 
@@ -4085,28 +4084,22 @@
   return (*fn)(a);
 }
 
-Marking virtual functions as ``not_tail_called`` is an error:
+Marking virtual functions as ``not_tail_called`` will not have effect on the
+overriding functions of derived classes:
 
   .. code-block:: c++
 
-class Base {
-public:
-  // not_tail_called on a virtual function is an error.
-  [[clang::not_tail_called]] virtual int foo1();
-
-  virtual int foo2();
-
-  // Non-virtual functions can be marked ``not_tail_called``.
-  [[clang::not_tail_called]] int foo3();
-};
-
-class Derived1 : public Base {
-public:
-  int foo1() override;
-
-  // not_tail_called on a virtual function is an error.
-  [[clang::not_tail_called]] int foo2() override;
+struct Foo { virtual void f(); };
+struct Bar : Foo {
+  [[clang::not_tail_called]] void f() override;
 };
+void callera(Bar& bar) {
+  Foo& foo = bar;
+  // not_tail_called will not have effect on here, even though the
+  // underlying method is f 

[PATCH] D97364: [docs] Add a release note for the removing of -Wreturn-std-move-in-c++11

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert accepted this revision.
aaronpuchert added a comment.

I assume you need someone to land this for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97364

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

[PATCH] D97204: [RFC] Clang 64-bit source locations

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

rsmith added a comment.

Thanks for doing this!

The 8-9% memory hit is better than I'd feared, but still seems uncomfortably 
large. I've left comments on a couple of places where I think we could 
substantially reduce this.
The performance regression seems large enough that people will notice, but 
perhaps not prohibitively huge. I assume that's all caused by increasing the 
size of the working set.

Can we avoid a libclang ABI break if we don't allow the use of 64-bit source 
locations for builds with 32-bit pointers?

It would be useful to get data points from more users.




Comment at: clang/include/clang/AST/DeclBase.h:1763-1784
+static_assert(sizeof(DeclContextBitfields) <= 16,
+  "DeclContextBitfields is larger than 16 bytes!");
+static_assert(sizeof(TagDeclBitfields) <= 16,
+  "TagDeclBitfields is larger than 16 bytes!");
+static_assert(sizeof(EnumDeclBitfields) <= 16,
+  "EnumDeclBitfields is larger than 16 bytes!");
+static_assert(sizeof(RecordDeclBitfields) <= 16,

I think allowing these to grow is likely a bad tradeoff -- we're paying 8 bytes 
per declaration in order to make only `ObjCContainerDecl`s smaller (actually, 
not even that -- there'll be 4 bytes of padding in an `ObjCContainerDecl` 
either way in 64-bit-`SourceLocation` mode). Can you try moving the 
`SourceLocation` out of `ObjCContainerDeclBitfields` and into 
`ObjCContainerDecl` and see if that makes a noticeable difference to the memory 
overhead of this patch?



Comment at: clang/include/clang/AST/Stmt.h:1154
   Stmt(StmtClass SC) {
-static_assert(sizeof(*this) <= 8,
+static_assert(sizeof(*this) <= 16,
   "changing bitfields changed sizeof(Stmt)");

Oof, we're wasting a lot of space here. Lots of `Stmt` subclasses only need a 
couple of bytes of bitfields, but we're padding them all out to 16 because some 
of them store a `SourceLocation`. It'd be nice to estimate how much of that we 
could get back by moving source locations to the derived classes, if you're 
prepared to do the work there. (I suspect that would result in too many 
`#ifdef`s to be reasonable to check in, though.)



Comment at: clang/include/clang/Analysis/ProgramPoint.h:90-111
+  union PtrOrSLoc {
+PtrOrSLoc() {}
+PtrOrSLoc(const void *P) {
+  memset(Raw.buffer, 0, sizeof(Raw.buffer));
+  Ptr = P;
+}
+PtrOrSLoc(SourceLocation L) {

Can we reasonably say that 32-bit clang builds don't support 64-bit 
`SourceLocation`s, and keep using a `void*` (or `uintptr_t`) here?



Comment at: clang/lib/Serialization/ASTReader.cpp:3893-3894
 
-uint32_t SLocOffset =
-endian::readNext(Data);
+SourceLocation::UIntType SLocOffset =
+endian::readNext(Data);
 uint32_t IdentifierIDOffset =

I think we should ensure the on-disk AST file format doesn't depend on the new 
macro. Going to 64 bits unconditionally here is probably fine.


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

https://reviews.llvm.org/D97204

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

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I expected nodebug already applied to types, even though the documentation says 
it only affects variables and functions. We should update the docs.

I think if we already have `nodebug` spelling, we don't need to make something 
general with modes. The "always emit" use case is fairly weak. It's easy to 
drop in a `static_assert(sizeof(Ty) > 0, "emit type info");` to get type info 
if you want it.

If we're looking at a no-argument attribute, my naming ideas lean towards 
incorporating "required [to be] complete" in the name. There are many ways that 
one can "use" or "require" a type that work fine with a forward declaration. 
Something like `emit_debug_typeinfo_if_required_complete`? Still too big? 
`emit_debug_if_required_complete`?




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2402-2405
+  // Don't omit definition if marked with attribute.
+  if (RD->hasAttr())
+return false;
+

dblaikie wrote:
> Perahps this should be a bit further up in this function? For instance the 
> template specialization homing seems like it'd override this behavior. 
> Perhaps someone has a template specialization that shouldn't be homed for 
> some reason? & similarly with modular debug info (both the 
> "isDefinediInClangModule" and "hasExternalDefinitions" cases are different 
> kinds of modular debug info homing)
> 
> Hmm, I guess the modular debug info is less likely to have these sort of 
> problems - it's more explicit about what is homed, both explicitly making a 
> home, and explicitly communicating the presence of a home to other 
> compilations that rely on that data. So it might be unfortunate to pessimize 
> that scenario unnecessarily.
> 
> @rnk - any thoughts on the tradeoff of uniformity of the attribute (ie: 
> applies to all type homing strategies) V applying the attribute to address 
> shortcomings in the source that only affect some homing strategies and not 
> others?
I think it makes sense to move this up so that the user can work around extern 
template type homing, but we probably don't need to override module type homing 
behavior.

I think if we're using modular debug info, we can be pretty confident that the 
module will provide debug info, but it's easy to construct scenarios where the 
extern template is provided by a TU that doesn't enable debug info. This 
attribute would allow the user to work around that without going all the way to 
enabling fstandalone-debug, which typically generates too much data to be 
usable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411

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

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Markus Böck 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 rG9f1b832331e3: Reland [Driver][Windows] Support 
per-target runtimes dir layout for profile… (authored by zero9178).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96638

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/BareMetal.h
  clang/test/Driver/cl-options.c
  clang/test/Driver/fsanitize.c
  clang/test/Driver/instrprof-ld.c
  clang/test/Driver/sanitizer-ld.c

Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -666,16 +666,16 @@
 // RUN: -target x86_64-pc-windows \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CFI-STATS-WIN64 %s
-// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats_client-x86_64.lib"
-// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats-x86_64.lib"
+// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats_client{{(-x86_64)?}}.lib"
+// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats{{(-x86_64)?}}.lib"
 // CHECK-CFI-STATS-WIN64: "--linker-option=/include:__sanitizer_stats_register"
 
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target i686-pc-windows \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CFI-STATS-WIN32 %s
-// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats_client-i386.lib"
-// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats-i386.lib"
+// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats_client{{(-i386)?}}.lib"
+// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats{{(-i386)?}}.lib"
 // CHECK-CFI-STATS-WIN32: "--linker-option=/include:___sanitizer_stats_register"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
Index: clang/test/Driver/instrprof-ld.c
===
--- clang/test/Driver/instrprof-ld.c
+++ clang/test/Driver/instrprof-ld.c
@@ -112,7 +112,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-I386 %s
 //
 // CHECK-WINDOWS-I386: "{{.*}}link{{(.exe)?}}"
-// CHECK-WINDOWS-I386: "{{.*}}clang_rt.profile-i386.lib"
+// CHECK-WINDOWS-I386: "{{.*}}clang_rt.profile{{(-i386)?}}.lib"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-pc-win32 -fprofile-instr-generate \
@@ -120,7 +120,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-X86-64 %s
 //
 // CHECK-WINDOWS-X86-64: "{{.*}}link{{(.exe)?}}"
-// CHECK-WINDOWS-X86-64: "{{.*}}clang_rt.profile-x86_64.lib"
+// CHECK-WINDOWS-X86-64: "{{.*}}clang_rt.profile{{(-x86_64)?}}.lib"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-mingw32 -fprofile-instr-generate -fuse-ld=ld \
@@ -136,7 +136,7 @@
 // RUN: -fprofile-instr-generate 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-X86-64-DEPENDENT-LIB %s
 //
-// CHECK-WINDOWS-X86-64-DEPENDENT-LIB: "--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
+// CHECK-WINDOWS-X86-64-DEPENDENT-LIB: "--dependent-lib={{[^"]*}}clang_rt.profile{{[^"]*}}.lib"
 //
 // RUN: %clang %s -### -o %t.o -target x86_64-mingw32 \
 // RUN: -fprofile-instr-generate 2>&1 \
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -20,17 +20,17 @@
 // RUN: %clang -target x86_64-pc-win32 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-UNDEFINED-WIN64,CHECK-UNDEFINED-MSVC
 // RUN: %clang -target x86_64-w64-mingw32 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-WIN64-MINGW
 // RUN: %clang -target x86_64-pc-win32 -fsanitize=undefined -x c++ %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-UNDEFINED-WIN64,CHECK-UNDEFINED-WIN-CXX,CHECK-UNDEFINED-MSVC
-// CHECK-UNDEFINED-WIN32: "--dependent-lib={{[^"]*}}ubsan_standalone-i386.lib"
-// CHECK-UNDEFINED-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
-// CHECK-UNDEFINED-WIN64-MINGW: "--dependent-lib={{[^"]*}}libclang_rt.ubsan_standalone-x86_64.a"
+// CHECK-UNDEFINED-WIN32: "--dependent-lib={{[^"]*}}ubsan_standalone{{(-i386)?}}.lib"
+// CHECK-UNDEFINED-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone{{(-x86_64)?}}.lib"
+// CHECK-UNDEFINED-WIN64-MINGW: "--dependent-lib={{[^"]*}}libclang_rt.ubsan_standalone{{(-x86_64)?}}.a"
 // CHECK-UNDEFINED-WIN-CXX: "--dependent-lib={{[^"]*}}ubsan_standalone_cxx{{[^"]*}}.lib"
 // CHECK-UNDEFINED-MSVC-SAME: 

[clang] 9f1b832 - Reland "[Driver][Windows] Support per-target runtimes dir layout for profile instr generate"

[Markus Böck via cfe-commits]

Author: Markus Böck
Date: 2021-02-24T23:40:20+01:00
New Revision: 9f1b832331e350426f7f2f8cc30ab8ba991f5884

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

LOG: Reland "[Driver][Windows] Support per-target runtimes dir layout for 
profile instr generate"

This relands commit rG7f9d5d6e444c which was reverted in rGab5b00ada9e7

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

Added: 


Modified: 
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChain.cpp
clang/lib/Driver/ToolChains/BareMetal.cpp
clang/lib/Driver/ToolChains/BareMetal.h
clang/test/Driver/cl-options.c
clang/test/Driver/fsanitize.c
clang/test/Driver/instrprof-ld.c
clang/test/Driver/sanitizer-ld.c

Removed: 




diff  --git a/clang/include/clang/Driver/ToolChain.h 
b/clang/include/clang/Driver/ToolChain.h
index df4848d9e1345..e7e5a1f7a6ad0 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -184,6 +184,11 @@ class ToolChain {
   virtual Tool *buildStaticLibTool() const;
   virtual Tool *getTool(Action::ActionClass AC) const;
 
+  virtual std::string buildCompilerRTBasename(const llvm::opt::ArgList ,
+  StringRef Component,
+  FileType Type,
+  bool AddArch) const;
+
   /// \name Utilities for implementing subclasses.
   ///@{
   static void addSystemInclude(const llvm::opt::ArgList ,
@@ -432,10 +437,9 @@ class ToolChain {
   getCompilerRTArgString(const llvm::opt::ArgList , StringRef Component,
  FileType Type = ToolChain::FT_Static) const;
 
-  virtual std::string
-  getCompilerRTBasename(const llvm::opt::ArgList , StringRef Component,
-FileType Type = ToolChain::FT_Static,
-bool AddArch = true) const;
+  std::string getCompilerRTBasename(const llvm::opt::ArgList ,
+StringRef Component,
+FileType Type = ToolChain::FT_Static) 
const;
 
   // Returns target specific runtime path if it exists.
   virtual Optional getRuntimePath() const;

diff  --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 372be613b795c..3f500617d8434 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -414,8 +414,16 @@ std::string ToolChain::getCompilerRTPath() const {
 }
 
 std::string ToolChain::getCompilerRTBasename(const ArgList ,
- StringRef Component, FileType 
Type,
- bool AddArch) const {
+ StringRef Component,
+ FileType Type) const {
+  std::string CRTAbsolutePath = getCompilerRT(Args, Component, Type);
+  return llvm::sys::path::filename(CRTAbsolutePath).str();
+}
+
+std::string ToolChain::buildCompilerRTBasename(const llvm::opt::ArgList ,
+   StringRef Component,
+   FileType Type,
+   bool AddArch) const {
   const llvm::Triple  = getTriple();
   bool IsITANMSVCWindows =
   TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment();
@@ -431,8 +439,8 @@ std::string ToolChain::getCompilerRTBasename(const ArgList 
,
 Suffix = IsITANMSVCWindows ? ".lib" : ".a";
 break;
   case ToolChain::FT_Shared:
-Suffix = Triple.isOSWindows()
- ? (Triple.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
+Suffix = TT.isOSWindows()
+ ? (TT.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
  : ".so";
 break;
   }
@@ -450,7 +458,7 @@ std::string ToolChain::getCompilerRT(const ArgList , 
StringRef Component,
  FileType Type) const {
   // Check for runtime files in the new layout without the architecture first.
   std::string CRTBasename =
-  getCompilerRTBasename(Args, Component, Type, /*AddArch=*/false);
+  buildCompilerRTBasename(Args, Component, Type, /*AddArch=*/false);
   for (const auto  : getLibraryPaths()) {
 SmallString<128> P(LibPath);
 llvm::sys::path::append(P, CRTBasename);
@@ -460,7 +468,8 @@ std::string ToolChain::getCompilerRT(const ArgList , 
StringRef Component,
 
   // Fall back to the old expected compiler-rt name if the new one does not
   // exist.
-  CRTBasename = getCompilerRTBasename(Args, Component, Type, /*AddArch=*/true);
+  CRTBasename =
+  buildCompilerRTBasename(Args, Component, Type, /*AddArch=*/true);
   SmallString<128> Path(getCompilerRTPath());
   

[PATCH] D97417: [clangd] use a compatible preamble for the first AST built

[Quentin Chateau via Phabricator via cfe-commits]

qchateau created this revision.
qchateau added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, javed.absar.
qchateau requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Keep a store of the preambles of all opened filed, plus up to
5 previously used preambles. When building the AST for a TU,
if the preamble for this TU is not yet built, look through
the stored premables to find the best compatible preamble
and use this preamble to speed-up the AST build.

-

This patch is starting to look like something acceptable, so
I'm sending it for review. At this point I'd like to get some 
feedback on this before going further.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97417

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h

Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -310,6 +310,8 @@
   /// Responsible for retaining and rebuilding idle ASTs. An implementation is
   /// an LRU cache.
   class ASTCache;
+  /// Store all known preambles
+  class PreambleStore;
 
   // The file being built/processed in the current thread. This is a hack in
   // order to get the file name into the index implementations. Do not depend on
@@ -332,6 +334,7 @@
   Semaphore QuickRunBarrier;
   llvm::StringMap> Files;
   std::unique_ptr IdleASTs;
+  std::unique_ptr Preambles;
   // None when running tasks synchronously and non-None when running tasks
   // asynchronously.
   llvm::Optional PreambleTasks;
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -94,6 +94,21 @@
 
 namespace {
 class ASTWorker;
+
+bool compileCommandsAreSimilar(const tooling::CompileCommand ,
+   const tooling::CompileCommand ) {
+  std::vector LHSCL(LHS.CommandLine);
+  auto LHSBasename = llvm::sys::path::filename(LHS.Filename).str();
+  auto RHSBasename = llvm::sys::path::filename(RHS.Filename).str();
+  for (auto  : LHSCL) {
+for (auto Pos = Arg.find(LHSBasename); Pos != std::string::npos;
+ Pos = Arg.find(LHSBasename, Pos + 1)) {
+  Arg.replace(Pos, LHSBasename.size(), RHSBasename);
+}
+  }
+  return llvm::makeArrayRef(LHSCL).equals(RHS.CommandLine);
+}
+
 } // namespace
 
 static clang::clangd::Key kFileBeingProcessed;
@@ -179,6 +194,59 @@
   std::vector LRU; /* GUARDED_BY(Mut) */
 };
 
+class TUScheduler::PreambleStore {
+public:
+  struct Entry {
+std::shared_ptr Preamble;
+size_t Score;
+Path FileName;
+
+bool operator>(const Entry ) const { return Score > Other.Score; }
+  };
+
+  auto getAll() {
+std::unique_lock Lock(Mut);
+return Store;
+  }
+
+  void push(PathRef FileName, std::shared_ptr Preamble) {
+std::unique_lock Lock(Mut);
+auto It = llvm::find_if(
+Store, [&](const auto ) { return Item.FileName == FileName; });
+if (It == Store.end()) {
+  Store.push_back(Entry{std::move(Preamble), 0, FileName.str()});
+  popWorstPreambles();
+}
+vlog("Store contains {0} preambles", Store.size());
+  }
+
+  void hit(PathRef FileName) {
+std::unique_lock Lock(Mut);
+auto It = llvm::find_if(
+Store, [&](const auto ) { return Item.FileName == FileName; });
+if (It == Store.end()) {
+  return;
+}
+It->Score++;
+  }
+
+private:
+  void popWorstPreambles() {
+constexpr int ClosedPreamblesToKeep = 5;
+auto Begin = llvm::partition(
+Store, [](const auto ) { return Item.Preamble.use_count() > 1; });
+if (std::distance(Begin, Store.end()) <= ClosedPreamblesToKeep) {
+  return;
+}
+auto Nth = Begin + ClosedPreamblesToKeep;
+std::nth_element(Begin, Nth, Store.end(), std::greater());
+Store.erase(Nth, Store.end());
+  }
+
+  std::mutex Mut;
+  std::vector Store;
+};
+
 namespace {
 /// Threadsafe manager for updating a TUStatus and emitting it after each
 /// update.
@@ -368,8 +436,10 @@
 class ASTWorker {
   friend class ASTWorkerHandle;
   ASTWorker(PathRef FileName, const GlobalCompilationDatabase ,
-TUScheduler::ASTCache , Semaphore , bool RunSync,
-const TUScheduler::Options , ParsingCallbacks );
+TUScheduler::ASTCache ,
+TUScheduler::PreambleStore , Semaphore ,
+bool RunSync, const TUScheduler::Options ,
+ParsingCallbacks );
 
 public:
   /// Create a new ASTWorker and return a handle to it.
@@ -377,12 +447,11 @@
   /// is null, all requests will be processed on the calling thread
   /// synchronously instead. \p Barrier is acquired when processing each
   /// request, it is used to limit 

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm


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

https://reviews.llvm.org/D96638

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

[PATCH] D95984: [CodeGen] Fix codegen for __attribute__((swiftasynccall)).

[Varun Gandhi via Phabricator via cfe-commits]

varungandhi-apple updated this revision to Diff 326203.
varungandhi-apple added a comment.

Generate tail call when doing codegen for return statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95984

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/64bit-swiftcall.c
  clang/test/CodeGen/arm-swiftcall.c
  clang/test/CodeGen/swift-async-call-conv.c
  clang/test/CodeGen/swift-call-conv.c

Index: clang/test/CodeGen/swift-call-conv.c
===
--- clang/test/CodeGen/swift-call-conv.c
+++ clang/test/CodeGen/swift-call-conv.c
@@ -6,3 +6,5 @@
 void __attribute__((__swiftcall__)) f(void) {}
 // CHECK-LABEL: define dso_local swiftcc void @f()
 
+void __attribute__((__swiftasynccall__)) f_async(void) {}
+// CHECK-LABEL: define dso_local swifttailcc void @f_async()
Index: clang/test/CodeGen/swift-async-call-conv.c
===
--- /dev/null
+++ clang/test/CodeGen/swift-async-call-conv.c
@@ -0,0 +1,126 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -target-cpu core2 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm64-apple-ios9 -target-cpu cyclone -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple armv7-apple-darwin9 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple armv7s-apple-ios9 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple armv7k-apple-ios9 -emit-llvm -o - %s | FileCheck %s
+
+// Test tail call behavior when a swiftasynccall function is called
+// from another swiftasynccall function.
+
+#define SWIFTCALL __attribute__((swiftcall))
+#define SWIFTASYNCCALL __attribute__((swiftasynccall))
+#define ASYNC_CONTEXT __attribute__((swift_async_context))
+
+// CHECK-LABEL: swifttailcc void @async_leaf1(i8* swiftasync
+SWIFTASYNCCALL void async_leaf1(char * ASYNC_CONTEXT ctx) {
+  *ctx += 1;
+}
+
+// CHECK-LABEL: swifttailcc void @async_leaf2(i8* swiftasync
+SWIFTASYNCCALL void async_leaf2(char * ASYNC_CONTEXT ctx) {
+  *ctx += 2;
+}
+
+// CHECK-LABEL: swifttailcc void @async_branch
+// CHECK: tail call swifttailcc void @async_leaf1
+// CHECK-NEXT: ret void
+// CHECK: tail call swifttailcc void @async_leaf2
+// CHECK-NEXT: ret void
+SWIFTASYNCCALL void async_branch(_Bool b, char * ASYNC_CONTEXT ctx) {
+  if (b) {
+return async_leaf1(ctx);
+  } else {
+return async_leaf2(ctx);
+  }
+}
+
+// CHECK-LABEL: swifttailcc void @async_not_all_tail
+// CHECK-NOT:  tail call swifttailcc void @async_leaf1
+// CHECK:  call swifttailcc void @async_leaf1
+// CHECK-NOT:  ret void
+// CHECK:  tail call swifttailcc void @async_leaf2
+// CHECK-NEXT: ret void
+SWIFTASYNCCALL void async_not_all_tail(char * ASYNC_CONTEXT ctx) {
+  async_leaf1(ctx);
+  return async_leaf2(ctx);
+}
+
+// CHECK-LABEL: swifttailcc void @async_loop
+// CHECK: tail call swifttailcc void @async_leaf1
+// CHECK-NEXT: ret void
+// CHECK: tail call swifttailcc void @async_leaf2
+// CHECK-NEXT: ret void
+// CHECK: tail call swifttailcc void @async_loop
+// CHECK-NEXT: ret void
+SWIFTASYNCCALL void async_loop(unsigned u, char * ASYNC_CONTEXT ctx) {
+  if (u == 0) {
+return async_leaf1(ctx);
+  } else if (u == 1) {
+return async_leaf2(ctx);
+  }
+  return async_loop(u - 2, ctx);
+}
+
+// Forward-declaration + mutual recursion is okay.
+
+SWIFTASYNCCALL void async_mutual_loop2(unsigned u, char * ASYNC_CONTEXT ctx);
+
+// CHECK: swifttailcc void @async_mutual_loop
+// CHECK: tail call swifttailcc void @async_leaf
+// CHECK-NEXT: ret void
+// CHECK: tail call swifttailcc void @async_leaf
+// CHECK-NEXT: ret void
+// CHECK: tail call swifttailcc void @async_mutual_loop
+// CHECK-NEXT: ret void
+SWIFTASYNCCALL void async_mutual_loop1(unsigned u, char * ASYNC_CONTEXT ctx) {
+  if (u == 0) {
+return async_leaf1(ctx);
+  } else if (u == 1) {
+return async_leaf2(ctx);
+  }
+  return async_mutual_loop2(u - 2, ctx);
+}
+
+// CHECK: swifttailcc void @async_mutual_loop
+// CHECK: tail call swifttailcc void @async_leaf1
+// CHECK-NEXT: ret void
+// CHECK: tail call swifttailcc void @async_leaf2
+// CHECK-NEXT: ret void
+// CHECK: tail call swifttailcc void @async_mutual_loop1
+// CHECK-NEXT: ret void
+SWIFTASYNCCALL void async_mutual_loop2(unsigned u, char * ASYNC_CONTEXT ctx) {
+  if (u == 0) {
+return async_leaf1(ctx);
+  } else if (u == 1) {
+return async_leaf2(ctx);
+  }
+  return async_mutual_loop1(u - 2, ctx);
+}
+
+// When swiftasynccall functions are called by non-swiftasynccall functions,
+// the call isn't marked as a tail call.
+
+// CHECK-LABEL: swiftcc i8 @sync_calling_async
+// CHECK-NOT: tail call
+// CHECK: call swifttailcc void @async_branch
+// CHECK-NOT: tail call
+// CHECK: call swifttailcc void @async_loop
+SWIFTCALL char sync_calling_async(_Bool b, unsigned u) {
+  char x = 'a';
+  async_branch(b, );
+  async_loop(u, );
+  

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Markus Böck via Phabricator via cfe-commits]

zero9178 updated this revision to Diff 326200.
zero9178 added a comment.

Avoid going through the library directories, checking for the existence of a 
runtime library, twice by having getCompilerRTBasename call getCompilerRT and 
simply extract the filename component. To allow other ToolChains to override 
the names of runtime libraries, getCompilerRT now calls the virtual function 
buildCompilerRTBasename. This is used by the BareMetal toolchain.

I chose the name buildCompilerRTBasename as the call chain 
getCompilerRTBasename -> getCompilerRT -> getCompilerRTBasenameImpl would be a 
bit confusing. I am not too attached to the name though if better suggestions 
come along.


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

https://reviews.llvm.org/D96638

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/BareMetal.h
  clang/test/Driver/cl-options.c
  clang/test/Driver/fsanitize.c
  clang/test/Driver/instrprof-ld.c
  clang/test/Driver/sanitizer-ld.c

Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -666,16 +666,16 @@
 // RUN: -target x86_64-pc-windows \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CFI-STATS-WIN64 %s
-// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats_client-x86_64.lib"
-// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats-x86_64.lib"
+// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats_client{{(-x86_64)?}}.lib"
+// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats{{(-x86_64)?}}.lib"
 // CHECK-CFI-STATS-WIN64: "--linker-option=/include:__sanitizer_stats_register"
 
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target i686-pc-windows \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CFI-STATS-WIN32 %s
-// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats_client-i386.lib"
-// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats-i386.lib"
+// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats_client{{(-i386)?}}.lib"
+// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats{{(-i386)?}}.lib"
 // CHECK-CFI-STATS-WIN32: "--linker-option=/include:___sanitizer_stats_register"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
Index: clang/test/Driver/instrprof-ld.c
===
--- clang/test/Driver/instrprof-ld.c
+++ clang/test/Driver/instrprof-ld.c
@@ -112,7 +112,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-I386 %s
 //
 // CHECK-WINDOWS-I386: "{{.*}}link{{(.exe)?}}"
-// CHECK-WINDOWS-I386: "{{.*}}clang_rt.profile-i386.lib"
+// CHECK-WINDOWS-I386: "{{.*}}clang_rt.profile{{(-i386)?}}.lib"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-pc-win32 -fprofile-instr-generate \
@@ -120,7 +120,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-X86-64 %s
 //
 // CHECK-WINDOWS-X86-64: "{{.*}}link{{(.exe)?}}"
-// CHECK-WINDOWS-X86-64: "{{.*}}clang_rt.profile-x86_64.lib"
+// CHECK-WINDOWS-X86-64: "{{.*}}clang_rt.profile{{(-x86_64)?}}.lib"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-mingw32 -fprofile-instr-generate -fuse-ld=ld \
@@ -136,7 +136,7 @@
 // RUN: -fprofile-instr-generate 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-X86-64-DEPENDENT-LIB %s
 //
-// CHECK-WINDOWS-X86-64-DEPENDENT-LIB: "--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
+// CHECK-WINDOWS-X86-64-DEPENDENT-LIB: "--dependent-lib={{[^"]*}}clang_rt.profile{{[^"]*}}.lib"
 //
 // RUN: %clang %s -### -o %t.o -target x86_64-mingw32 \
 // RUN: -fprofile-instr-generate 2>&1 \
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -20,17 +20,17 @@
 // RUN: %clang -target x86_64-pc-win32 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-UNDEFINED-WIN64,CHECK-UNDEFINED-MSVC
 // RUN: %clang -target x86_64-w64-mingw32 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-WIN64-MINGW
 // RUN: %clang -target x86_64-pc-win32 -fsanitize=undefined -x c++ %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-UNDEFINED-WIN64,CHECK-UNDEFINED-WIN-CXX,CHECK-UNDEFINED-MSVC
-// CHECK-UNDEFINED-WIN32: "--dependent-lib={{[^"]*}}ubsan_standalone-i386.lib"
-// CHECK-UNDEFINED-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
-// CHECK-UNDEFINED-WIN64-MINGW: "--dependent-lib={{[^"]*}}libclang_rt.ubsan_standalone-x86_64.a"
+// CHECK-UNDEFINED-WIN32: "--dependent-lib={{[^"]*}}ubsan_standalone{{(-i386)?}}.lib"
+// CHECK-UNDEFINED-WIN64: 

[PATCH] D97138: [clang][flang] Improve the consistency of the code-base

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added inline comments.



Comment at: clang/tools/driver/driver.cpp:349
 "source, and associated run script.\n");
-  SmallVector argv(argv_, argv_ + argc_);
+  SmallVector ArgValues(Argv, Argv + Argc);
 

What about just `Args`? "Values" sounds a bit too broad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97138

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

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Generally OK with this, as it seemed the libc++ issue might be a bit thorny to 
solve. Though I'd like to hear from libc++ maintainers about how they feel to 
ensure they're comfortable adding this attribute in the interim (or if this 
might be a motivation to fix libc++ instead of adding the attribute)

As for the name - I take it the "if required" is meant to connote the fact that 
this attribute only has an effect if the type is used/reachable from the DWARF, 
and doesn't force the type to be emitted even when unused/unreferenced?

Maybe "full_debug_if_used"? or "debug_full_if_used" (or maybe even reuse the 
equivalent compiler flag: standalone_debug?)

In D97411#2585910 , @rnk wrote:

> To help bikeshed the name, I can imagine a few other use cases:
>
> - an attribute to suppress type info emission, never emit full type info for 
> this type (similar to nodebug / artificial attributes for functions) -- this 
> could help optimize debug info size

nodebug is already supported on types for this purpose - we used it in libc++ 
to remove some type trait debug info to trim it down.

> - an attribute to always emit type information, whether it is required to be 
> complete or not (similar to -fno-eliminate-unused-types behavior)
> - non-use-case: It's not clear if it's useful to have an attribute to 
> explicitly indicate that a type should use the vptr, constructor, or extern 
> template heuristics.
>
> I thought maybe we could have a single attribute that takes a mode, something 
> like 
> `emit_debug_type_info("never/always/when_required_complete/with_ctor/with_vtable")`.
>  Or maybe shorter is better: `emit_typeinfo("when_required_complete")`. But 
> that sounds like we're talking about RTTI, not debug info.

Yeah, might be useful to have something that supports the different type homing 
strategies separately, though it is a more complicated user facing feature. The 
"always" mode could be handy, though I hesitate to build more features/surface 
area without users in mind, especially since these sort of things may paper 
over debug info issues we might be better off fixing generally.

I'd like to keep "debug" or "debuginfo" in the name, as you say, to avoid some 
confusion with RTTI for instance.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2402-2405
+  // Don't omit definition if marked with attribute.
+  if (RD->hasAttr())
+return false;
+

Perahps this should be a bit further up in this function? For instance the 
template specialization homing seems like it'd override this behavior. Perhaps 
someone has a template specialization that shouldn't be homed for some reason? 
& similarly with modular debug info (both the "isDefinediInClangModule" and 
"hasExternalDefinitions" cases are different kinds of modular debug info homing)

Hmm, I guess the modular debug info is less likely to have these sort of 
problems - it's more explicit about what is homed, both explicitly making a 
home, and explicitly communicating the presence of a home to other compilations 
that rely on that data. So it might be unfortunate to pessimize that scenario 
unnecessarily.

@rnk - any thoughts on the tradeoff of uniformity of the attribute (ie: applies 
to all type homing strategies) V applying the attribute to address shortcomings 
in the source that only affect some homing strategies and not others?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411

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

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

Thanks for the update @vvereschaka ! Just a few minor things and it should be 
good to go.
Also please use `git diff -U9` next time to add context to the patch.




Comment at: clang/docs/UsersManual.rst:783
   It is possible to specify this option without any value. In this case 
statistics
   is printed on standard output in human readable format:
   

I think this should read //"In this case statistics **are** printed.."// Would 
you mind changing this as well?



Comment at: clang/docs/UsersManual.rst:799
+  setting the ``CC_PRINT_PROC_STAT`` and ``CC_PRINT_PROC_STAT_FILE`` 
environment
+  variables. Use ``CC_PRINT_PROC_STAT_FILE`` to provide a file name to store
+  the statistics.

Do you think it would be possible to rephrase a bit to avoid the repetition of 
`CC_PRINT_PROC_STAT_FILE`? Perhaps also explicitate that `CC_PRINT_PROC_STAT` 
is for "enabling the feature and logging to stdout"; while 
`CC_PRINT_PROC_STAT_FILE` only "redirects the log to a file".



Comment at: clang/lib/Driver/Driver.cpp:1108
+CCPrintProcessStats = true;
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report);
+

Remove this too, `hasArg` claims all arguments.



Comment at: clang/lib/Driver/Driver.cpp:4039
+
+  if (CCPrintStatReportFilename == nullptr) {
 using namespace llvm;

Suggestion: I find `if (!CCPrintStatReportFilename)` more intutive, more 
compact and easier to read, but that's my personal preference. There are 
arguments both ways probably, up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97094

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

[PATCH] D95561: [Clang] Introduce Swift async calling convention.

[Varun Gandhi via Phabricator via cfe-commits]

varungandhi-apple updated this revision to Diff 326194.
varungandhi-apple added a comment.

Address review feedback; remove centralized target check for CC_SwiftAsync.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95561

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/CodeGen/SwiftCallingConv.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/arm-swiftcall.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGen/swift-call-conv.c
  clang/test/Sema/attr-c2x.c
  clang/test/Sema/attr-swiftcall.c
  clang/test/Sema/no_callconv.cpp
  clang/test/SemaCXX/attr-swiftcall.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
  llvm/lib/Demangle/MicrosoftDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
  llvm/test/Demangle/ms-mangle.test

Index: llvm/test/Demangle/ms-mangle.test
===
--- llvm/test/Demangle/ms-mangle.test
+++ llvm/test/Demangle/ms-mangle.test
@@ -341,6 +341,9 @@
 ?swift_func@@YSXXZ
 ; CHECK: void __attribute__((__swiftcall__)) swift_func(void)
 
+?swift_async_func@@YTXXZ
+; CHECK: void __attribute__((__swiftasynccall__)) swift_async_func(void)
+
 ??$fn_tmpl@$1?extern_c_func@@YAXXZ@@YAXXZ
 ; CHECK: void __cdecl fn_tmpl< __cdecl extern_c_func(void)>(void)
 
Index: llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
===
--- llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
+++ llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
@@ -110,6 +110,9 @@
   case CallingConv::Swift:
 OS << "__attribute__((__swiftcall__)) ";
 break;
+  case CallingConv::SwiftAsync:
+OS << "__attribute__((__swiftasynccall__)) ";
+break;
   default:
 break;
   }
Index: llvm/lib/Demangle/MicrosoftDemangle.cpp
===
--- llvm/lib/Demangle/MicrosoftDemangle.cpp
+++ llvm/lib/Demangle/MicrosoftDemangle.cpp
@@ -1713,6 +1713,8 @@
 return CallingConv::Vectorcall;
   case 'S':
 return CallingConv::Swift;
+  case 'T':
+return CallingConv::SwiftAsync;
   }
 
   return CallingConv::None;
Index: llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
===
--- llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
+++ llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
@@ -67,7 +67,8 @@
   Eabi,
   Vectorcall,
   Regcall,
-  Swift, // Clang-only
+  Swift,  // Clang-only
+  SwiftAsync, // Clang-only
 };
 
 enum class ReferenceKind : uint8_t { None, LValueRef, RValueRef };
Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -664,6 +664,7 @@
   TCALLINGCONV(AAPCS_VFP);
   TCALLINGCONV(IntelOclBicc);
   TCALLINGCONV(Swift);
+  TCALLINGCONV(SwiftAsync);
   TCALLINGCONV(PreserveMost);
   TCALLINGCONV(PreserveAll);
 case CC_SpirFunction: return CXCallingConv_Unexposed;
Index: clang/test/SemaCXX/attr-swiftcall.cpp
===
--- clang/test/SemaCXX/attr-swiftcall.cpp
+++ clang/test/SemaCXX/attr-swiftcall.cpp
@@ -1,14 +1,20 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify %s
 
 #define SWIFTCALL __attribute__((swiftcall))
+#define SWIFTASYNCCALL __attribute__((swiftasynccall))
 #define INDIRECT_RESULT __attribute__((swift_indirect_result))
 #define ERROR_RESULT __attribute__((swift_error_result))
 #define CONTEXT __attribute__((swift_context))
+#define ASYNC_CONTEXT __attribute__((swift_async_context))
 
 int notAFunction SWIFTCALL; // expected-warning {{'swiftcall' only applies to function types; type here is 'int'}}
+int notAnAsyncFunction SWIFTASYNCCALL; // expected-warning {{'swiftasynccall' only applies to function types; type here is 'int'}}
 void variadic(int x, ...) SWIFTCALL; // expected-error {{variadic function cannot use swiftcall calling convention}}
+void variadic_async(int x, ...) SWIFTASYNCCALL; // expected-error {{variadic function cannot use swiftasynccall calling convention}}
 void multiple_ccs(int x) SWIFTCALL __attribute__((vectorcall)); // expected-error {{vectorcall and swiftcall attributes are not 

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

To help bikeshed the name, I can imagine a few other use cases:

- an attribute to suppress type info emission, never emit full type info for 
this type (similar to nodebug / artificial attributes for functions) -- this 
could help optimize debug info size
- an attribute to always emit type information, whether it is required to be 
complete or not (similar to -fno-eliminate-unused-types behavior)
- non-use-case: It's not clear if it's useful to have an attribute to 
explicitly indicate that a type should use the vptr, constructor, or extern 
template heuristics.

I thought maybe we could have a single attribute that takes a mode, something 
like 
`emit_debug_type_info("never/always/when_required_complete/with_ctor/with_vtable")`.
 Or maybe shorter is better: `emit_typeinfo("when_required_complete")`. But 
that sounds like we're talking about RTTI, not debug info.




Comment at: clang/include/clang/Basic/Attr.td:1666
+  let Subjects = SubjectList<[CXXRecord]>;
+  let Documentation = [Undocumented];
+  let SimpleHandler = 1;

I understand that this patch is mainly to open discussion, but we should 
document the attribute before landing the patch.



Comment at: clang/test/CodeGenCXX/force-debug-attribute.cpp:10-11
+
+// Struct that isn't constructed, so its full type info should be omitted with
+// -debug-info-kind=constructor.
+struct DEBUGASNEEDED some_struct {

This attribute should also work with vtable and extern template instantiation 
type homing, right? So for example, I would see this template type in the debug 
info in the standard compilation modes as well:
  template  struct Foo { Foo() {} T x; };
  extern template struct DEBUGASNEEDED Foo; // I expect to see a complete 
Foo DICompositeType


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411

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

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/Driver/ToolChain.cpp:468-469
   // Check for runtime files in the new layout without the architecture first.
-  std::string CRTBasename =
-  buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/false);
-  for (const auto  : getLibraryPaths()) {
-SmallString<128> P(LibPath);
-llvm::sys::path::append(P, CRTBasename);
-if (getVFS().exists(P))
-  return std::string(P.str());
+  std::string CRTBasename = getCompilerRTBasename(Args, Component, Type);
+  if (auto value = findInLibraryPath(*this, CRTBasename)) {
+return value.getValue();

zero9178 wrote:
> rnk wrote:
> > This does twice as many stat syscalls as we need. That seems worth 
> > optimizing. First, in getCompilerRTBasename, we search the library path, we 
> > return a result, and then we search it again here. I think the nicest 
> > solution is probably to have one shared implementation that takes a boolean 
> > and returns the full path or the basename.
> Refactoring the current implementation out of getCompilerRTBasename is 
> currently not possible as that would break the BareMetal toolchain as it 
> overrides getCompilerRTBasename to build up a different scheme for it's 
> runtime libraries. Not calling getCompilerRTBasename is what caused the test 
> failure previously.
> 
> Unless you mean that I should add an optional boolean operand to 
> getCompilerRTBasename that would get either the absolute path if possible or 
> always return a relative path? That could work and would change getCompilerRT 
> to not go through the library path but to instead check if that path is 
> absolute and return if it is already. 
> 
> 
> 
> 
I see, good point. I'd prefer to avoid extra boolean parameters when possible. 
My next best idea is to separate getCompilerRTBasename into two methods, one 
public non-virtual, and one protected virtual (getCompilerRTBasenameImpl? ech) 
to allow baremetal to override. The public one would do the obvious thing of 
returning sys::fs::filename of getCompilerRT. The protected one would retain 
the current implementation, and we'd update the bare metal toolchains to 
override the protected one.

We have prior history of doing excessive amounts of Linux Distro detection 
(D87187), which is why I'm being a bit fussy about the stat calls.


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

https://reviews.llvm.org/D96638

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

[PATCH] D65696: Implements CWG 2082 Referring to parameters in unevaluated operands of default arguments

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

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:143-148
+  /// CheckDefaultArgumentVisitorODR - C++ [dcl.fct.default] Traverses
+  /// the default argument of a parameter to determine whether it
+  /// contains ODR violations. These violations cannot be checked in
+  /// \ref CheckDefaultArgumentVisitor since the DeclRefExp's may be changed to
+  /// an implicit cast from an LValue to RValue by \ref 
SetParamDefaultArgument.
+  /// When that happens the ODR usage may be allowed.

Instead of visiting the default argument twice, can we call 
`SetParamDefaultArgument` first before doing the checks?


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

https://reviews.llvm.org/D65696

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

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[Amy Huang via Phabricator via cfe-commits]

akhuang created this revision.
akhuang added reviewers: rnk, dblaikie, rsmith.
Herald added a reviewer: aaron.ballman.
akhuang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This was motivated by the fact that constructor type homing (debug info
optimization that we want to turn on by default) drops some libc++ types,
so an attribute would allow us to override constructor homing and emit
them anyway. I'm currently looking into the particular libc++ issue, but
even if we do fix that, this issue might come up elsewhere and it might be
nice to have this.

As I've implemented it now, the attribute isn't specific to the
constructor homing optimization and overrides all of the debug info
optimizations.

Open to discussion about naming, specifics on what the attribute should do, etc.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97411

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/force-debug-attribute.cpp


Index: clang/test/CodeGenCXX/force-debug-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/force-debug-attribute.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -DSETATTR=0 -emit-llvm -std=c++14 
-debug-info-kind=constructor %s -o - | FileCheck %s --check-prefix=DEBUG
+// RUN: %clang_cc1 -DSETATTR=1 -emit-llvm -std=c++14 
-debug-info-kind=constructor %s -o - | FileCheck %s --check-prefix=WITHATTR
+
+#if SETATTR
+#define DEBUGASNEEDED __attribute__((force_debug_if_required_type))
+#else
+#define DEBUGASNEEDED
+#endif
+
+// Struct that isn't constructed, so its full type info should be omitted with
+// -debug-info-kind=constructor.
+struct DEBUGASNEEDED some_struct {
+  some_struct() {}
+};
+
+void func1(some_struct s) {}
+// void func2() { func1(); }
+// DEBUG:  !DICompositeType({{.*}}name: "some_struct"
+// DEBUG-SAME:  flags: {{.*}}DIFlagFwdDecl
+// WITHATTR: !DICompositeType({{.*}}name: "some_struct"
+// WITHATTR-NOT: DIFlagFwdDecl
+
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2399,6 +2399,10 @@
   if (!CXXDecl)
 return false;
 
+  // Don't omit definition if marked with attribute.
+  if (RD->hasAttr())
+return false;
+
   // Only emit complete debug info for a dynamic class when its vtable is
   // emitted.  However, Microsoft debuggers don't resolve type information
   // across DLL boundaries, so skip this optimization if the class or any of 
its
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1660,6 +1660,13 @@
   let Documentation = [NoDebugDocs];
 }
 
+def ForceDebugIfRequiredType : InheritableAttr {
+  let Spellings = [Clang<"force_debug_if_required_type">];
+  let Subjects = SubjectList<[CXXRecord]>;
+  let Documentation = [Undocumented];
+  let SimpleHandler = 1;
+}
+
 def NoDuplicate : InheritableAttr {
   let Spellings = [Clang<"noduplicate">];
   let Subjects = SubjectList<[Function]>;


Index: clang/test/CodeGenCXX/force-debug-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/force-debug-attribute.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -DSETATTR=0 -emit-llvm -std=c++14 -debug-info-kind=constructor %s -o - | FileCheck %s --check-prefix=DEBUG
+// RUN: %clang_cc1 -DSETATTR=1 -emit-llvm -std=c++14 -debug-info-kind=constructor %s -o - | FileCheck %s --check-prefix=WITHATTR
+
+#if SETATTR
+#define DEBUGASNEEDED __attribute__((force_debug_if_required_type))
+#else
+#define DEBUGASNEEDED
+#endif
+
+// Struct that isn't constructed, so its full type info should be omitted with
+// -debug-info-kind=constructor.
+struct DEBUGASNEEDED some_struct {
+  some_struct() {}
+};
+
+void func1(some_struct s) {}
+// void func2() { func1(); }
+// DEBUG:  !DICompositeType({{.*}}name: "some_struct"
+// DEBUG-SAME:  flags: {{.*}}DIFlagFwdDecl
+// WITHATTR: !DICompositeType({{.*}}name: "some_struct"
+// WITHATTR-NOT: DIFlagFwdDecl
+
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2399,6 +2399,10 @@
   if (!CXXDecl)
 return false;
 
+  // Don't omit definition if marked with attribute.
+  if (RD->hasAttr())
+return false;
+
   // Only emit complete debug info for a dynamic class when its vtable is
   // emitted.  However, Microsoft debuggers don't resolve type information
   // across DLL boundaries, so skip this optimization if the class or any of its
Index: clang/include/clang/Basic/Attr.td
===
--- 

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Something we should probably check is the interaction between -save-temps and 
whether we are trying to compile a single file or an executable, e.g. the 
difference between clang and clang -c.

If trying to compile foo.c directly to an executable, -save-temps should 
probably print (along with lots of others) an assembly file containing the 
contents of the code object (the shared elf containing amdgcn machine code). If 
trying to compile only the host part, we probably want to emit asm. If trying 
to compile only the target part, we probably don't - the llc part is likely to 
error if the rest of the application is missing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96769

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

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:4109
-  [[clang::not_tail_called]] int foo2() override;
-};
   }];

rnk wrote:
> aaron.ballman wrote:
> > rnk wrote:
> > > aaron.ballman wrote:
> > > > Quuxplusone wrote:
> > > > > aaron.ballman wrote:
> > > > > > ahatanak wrote:
> > > > > > > Quuxplusone wrote:
> > > > > > > > (Moving into a thread)
> > > > > > > > 
> > > > > > > > > This patch doesn't prevent the call to method in the code 
> > > > > > > > > below from being tail called,
> > > > > > > > > but I suppose users would expect the attribute to prevent the 
> > > > > > > > > tail call?
> > > > > > > > ```
> > > > > > > > struct B {
> > > > > > > >   virtual void method();  
> > > > > > > > };
> > > > > > > > struct D : B {
> > > > > > > >   [[clang::not_tail_called]] void method() override; 
> > > > > > > > };
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > The way virtual calls are handled in C++ is, all attributes and 
> > > > > > > > properties of the call are determined based on the //static// 
> > > > > > > > type at the call site; and then the //runtime destination// of 
> > > > > > > > the call is determined from the pointer in the vtable. 
> > > > > > > > Attributes and properties have no runtime existence, and so 
> > > > > > > > they physically cannot affect anything at runtime. Consider 
> > > > > > > > https://godbolt.org/z/P3799e :
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > struct Ba {
> > > > > > > >   virtual Ba *method(int x = 1);  
> > > > > > > > };
> > > > > > > > struct Da : Ba {
> > > > > > > >   [[clang::not_tail_called]] [[nodiscard]] Da *method(int x = 
> > > > > > > > 2) noexcept override; 
> > > > > > > > };
> > > > > > > > auto callera(Da& da) {
> > > > > > > > Ba& ba = da;
> > > > > > > > ba.method();
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > Here the call that is made is a //virtual// call (because 
> > > > > > > > `Ba::method` is virtual); with a default argument value of `1` 
> > > > > > > > (because `Ba::method`'s `x` parameter has a default value of 
> > > > > > > > `1`); and it returns something of type `Ba*` (because that's 
> > > > > > > > what `Ba::method` returns); and it is not considered to be 
> > > > > > > > noexcept (because `Ba::method` isn't marked noexcept); and it's 
> > > > > > > > okay to discard the result (because `Ba::method` is not 
> > > > > > > > nodiscard) and it is tail-called (because `Ba::method` doesn't 
> > > > > > > > disallow tail calls). All of these attributes and properties 
> > > > > > > > are based on the //static// type of variable `ba`, despite the 
> > > > > > > > fact that //at runtime// we'll end up jumping to the code for 
> > > > > > > > `Da::method`. According to the source code, statically, 
> > > > > > > > `Da::method` has a default argument of `2`, returns `Da*`, is 
> > > > > > > > noexcept, and is nodiscard, and disallows tail-calls. But we're 
> > > > > > > > not calling `da.method()`, we're calling `ba.method()`; so none 
> > > > > > > > of that matters to our call site at `callera`.
> > > > > > > > 
> > > > > > > > I think this patch is a good thing.
> > > > > > > OK, I see. I think this patch is fine then.
> > > > > > > 
> > > > > > > Should we add an explanation of how virtual functions are 
> > > > > > > handled? The doc currently just says the attribute prevents 
> > > > > > > tail-call optimization on statically bound calls.
> > > > > > > I think this patch is a good thing.
> > > > > > 
> > > > > > I agree with your explanation but I'm not certain I agree with your 
> > > > > > conclusion. :-) I think the examples you point out are more often a 
> > > > > > source of confusion for users than not because of the nature of 
> > > > > > static vs dynamic dispatch, and so saying "this behavior can be 
> > > > > > consistent with all of these other things people often get confused 
> > > > > > by" may be justifiable but also seems a bit user-hostile.
> > > > > > 
> > > > > > Taking a slightly different example:
> > > > > > ```
> > > > > > struct Ba {
> > > > > >   [[clang::not_tail_called]] virtual Ba *method();  
> > > > > > };
> > > > > > struct Da : Ba {
> > > > > >   Ba *method() override; 
> > > > > > };
> > > > > > 
> > > > > > void callera(Da& da) {
> > > > > > Ba& ba = da;
> > > > > > ba.method();
> > > > > > }
> > > > > > ```
> > > > > > There's no guarantee that `Ba::method()` and `Da::method()` have 
> > > > > > the same not-tail-called properties. The codegen for this case will 
> > > > > > still attach `notail` to the call site even though the dynamic 
> > > > > > target may not meet that requirement.
> > > > > > 
> > > > > > tl;dr: I think `notail` is a property of the call expression and 
> > > > > > the only way to know that's valid is when you know what's being 
> > > > > > called, so the current behavior is more user-friendly for avoiding 
> > > > > > optimization issues. 

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Markus Böck via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChain.cpp:468-469
   // Check for runtime files in the new layout without the architecture first.
-  std::string CRTBasename =
-  buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/false);
-  for (const auto  : getLibraryPaths()) {
-SmallString<128> P(LibPath);
-llvm::sys::path::append(P, CRTBasename);
-if (getVFS().exists(P))
-  return std::string(P.str());
+  std::string CRTBasename = getCompilerRTBasename(Args, Component, Type);
+  if (auto value = findInLibraryPath(*this, CRTBasename)) {
+return value.getValue();

rnk wrote:
> This does twice as many stat syscalls as we need. That seems worth 
> optimizing. First, in getCompilerRTBasename, we search the library path, we 
> return a result, and then we search it again here. I think the nicest 
> solution is probably to have one shared implementation that takes a boolean 
> and returns the full path or the basename.
Refactoring the current implementation out of getCompilerRTBasename is 
currently not possible as that would break the BareMetal toolchain as it 
overrides getCompilerRTBasename to build up a different scheme for it's runtime 
libraries. Not calling getCompilerRTBasename is what caused the test failure 
previously.

Unless you mean that I should add an optional boolean operand to 
getCompilerRTBasename that would get either the absolute path if possible or 
always return a relative path? That could work and would change getCompilerRT 
to not go through the library path but to instead check if that path is 
absolute and return if it is already. 






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

https://reviews.llvm.org/D96638

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

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Markus Böck via Phabricator via cfe-commits]

zero9178 updated this revision to Diff 326178.
zero9178 added a comment.

Addressed one of the reviewer requests and also uploaded additional files part 
of the patch that were accidently left out.


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

https://reviews.llvm.org/D96638

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/BareMetal.h
  clang/test/Driver/cl-options.c
  clang/test/Driver/fsanitize.c
  clang/test/Driver/instrprof-ld.c
  clang/test/Driver/sanitizer-ld.c

Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -666,16 +666,16 @@
 // RUN: -target x86_64-pc-windows \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CFI-STATS-WIN64 %s
-// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats_client-x86_64.lib"
-// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats-x86_64.lib"
+// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats_client{{(-x86_64)?}}.lib"
+// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats{{(-x86_64)?}}.lib"
 // CHECK-CFI-STATS-WIN64: "--linker-option=/include:__sanitizer_stats_register"
 
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target i686-pc-windows \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CFI-STATS-WIN32 %s
-// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats_client-i386.lib"
-// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats-i386.lib"
+// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats_client{{(-i386)?}}.lib"
+// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats{{(-i386)?}}.lib"
 // CHECK-CFI-STATS-WIN32: "--linker-option=/include:___sanitizer_stats_register"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
Index: clang/test/Driver/instrprof-ld.c
===
--- clang/test/Driver/instrprof-ld.c
+++ clang/test/Driver/instrprof-ld.c
@@ -112,7 +112,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-I386 %s
 //
 // CHECK-WINDOWS-I386: "{{.*}}link{{(.exe)?}}"
-// CHECK-WINDOWS-I386: "{{.*}}clang_rt.profile-i386.lib"
+// CHECK-WINDOWS-I386: "{{.*}}clang_rt.profile{{(-i386)?}}.lib"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-pc-win32 -fprofile-instr-generate \
@@ -120,7 +120,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-X86-64 %s
 //
 // CHECK-WINDOWS-X86-64: "{{.*}}link{{(.exe)?}}"
-// CHECK-WINDOWS-X86-64: "{{.*}}clang_rt.profile-x86_64.lib"
+// CHECK-WINDOWS-X86-64: "{{.*}}clang_rt.profile{{(-x86_64)?}}.lib"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-mingw32 -fprofile-instr-generate -fuse-ld=ld \
@@ -136,7 +136,7 @@
 // RUN: -fprofile-instr-generate 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-X86-64-DEPENDENT-LIB %s
 //
-// CHECK-WINDOWS-X86-64-DEPENDENT-LIB: "--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
+// CHECK-WINDOWS-X86-64-DEPENDENT-LIB: "--dependent-lib={{[^"]*}}clang_rt.profile{{[^"]*}}.lib"
 //
 // RUN: %clang %s -### -o %t.o -target x86_64-mingw32 \
 // RUN: -fprofile-instr-generate 2>&1 \
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -20,17 +20,17 @@
 // RUN: %clang -target x86_64-pc-win32 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-UNDEFINED-WIN64,CHECK-UNDEFINED-MSVC
 // RUN: %clang -target x86_64-w64-mingw32 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-WIN64-MINGW
 // RUN: %clang -target x86_64-pc-win32 -fsanitize=undefined -x c++ %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-UNDEFINED-WIN64,CHECK-UNDEFINED-WIN-CXX,CHECK-UNDEFINED-MSVC
-// CHECK-UNDEFINED-WIN32: "--dependent-lib={{[^"]*}}ubsan_standalone-i386.lib"
-// CHECK-UNDEFINED-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
-// CHECK-UNDEFINED-WIN64-MINGW: "--dependent-lib={{[^"]*}}libclang_rt.ubsan_standalone-x86_64.a"
+// CHECK-UNDEFINED-WIN32: "--dependent-lib={{[^"]*}}ubsan_standalone{{(-i386)?}}.lib"
+// CHECK-UNDEFINED-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone{{(-x86_64)?}}.lib"
+// CHECK-UNDEFINED-WIN64-MINGW: "--dependent-lib={{[^"]*}}libclang_rt.ubsan_standalone{{(-x86_64)?}}.a"
 // CHECK-UNDEFINED-WIN-CXX: "--dependent-lib={{[^"]*}}ubsan_standalone_cxx{{[^"]*}}.lib"
 // CHECK-UNDEFINED-MSVC-SAME: 

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/Driver/ToolChain.cpp:446
   case ToolChain::FT_Shared:
-Suffix = TT.isOSWindows()
- ? (TT.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
+Suffix = Triple.isOSWindows()
+ ? (Triple.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")

This is the same as `TT`, right? Please use TT here or remove TT and use Triple 
across this function. I guess I lean towards keeping TT, but it's up to you.



Comment at: clang/lib/Driver/ToolChain.cpp:468-469
   // Check for runtime files in the new layout without the architecture first.
-  std::string CRTBasename =
-  buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/false);
-  for (const auto  : getLibraryPaths()) {
-SmallString<128> P(LibPath);
-llvm::sys::path::append(P, CRTBasename);
-if (getVFS().exists(P))
-  return std::string(P.str());
+  std::string CRTBasename = getCompilerRTBasename(Args, Component, Type);
+  if (auto value = findInLibraryPath(*this, CRTBasename)) {
+return value.getValue();

This does twice as many stat syscalls as we need. That seems worth optimizing. 
First, in getCompilerRTBasename, we search the library path, we return a 
result, and then we search it again here. I think the nicest solution is 
probably to have one shared implementation that takes a boolean and returns the 
full path or the basename.


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

https://reviews.llvm.org/D96638

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

[PATCH] D96847: [clang][cli] Store additional optimization remarks info

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith 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/D96847/new/

https://reviews.llvm.org/D96847

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

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:4109
-  [[clang::not_tail_called]] int foo2() override;
-};
   }];

aaron.ballman wrote:
> rnk wrote:
> > aaron.ballman wrote:
> > > Quuxplusone wrote:
> > > > aaron.ballman wrote:
> > > > > ahatanak wrote:
> > > > > > Quuxplusone wrote:
> > > > > > > (Moving into a thread)
> > > > > > > 
> > > > > > > > This patch doesn't prevent the call to method in the code below 
> > > > > > > > from being tail called,
> > > > > > > > but I suppose users would expect the attribute to prevent the 
> > > > > > > > tail call?
> > > > > > > ```
> > > > > > > struct B {
> > > > > > >   virtual void method();  
> > > > > > > };
> > > > > > > struct D : B {
> > > > > > >   [[clang::not_tail_called]] void method() override; 
> > > > > > > };
> > > > > > > ```
> > > > > > > 
> > > > > > > The way virtual calls are handled in C++ is, all attributes and 
> > > > > > > properties of the call are determined based on the //static// 
> > > > > > > type at the call site; and then the //runtime destination// of 
> > > > > > > the call is determined from the pointer in the vtable. Attributes 
> > > > > > > and properties have no runtime existence, and so they physically 
> > > > > > > cannot affect anything at runtime. Consider 
> > > > > > > https://godbolt.org/z/P3799e :
> > > > > > > 
> > > > > > > ```
> > > > > > > struct Ba {
> > > > > > >   virtual Ba *method(int x = 1);  
> > > > > > > };
> > > > > > > struct Da : Ba {
> > > > > > >   [[clang::not_tail_called]] [[nodiscard]] Da *method(int x = 2) 
> > > > > > > noexcept override; 
> > > > > > > };
> > > > > > > auto callera(Da& da) {
> > > > > > > Ba& ba = da;
> > > > > > > ba.method();
> > > > > > > }
> > > > > > > ```
> > > > > > > Here the call that is made is a //virtual// call (because 
> > > > > > > `Ba::method` is virtual); with a default argument value of `1` 
> > > > > > > (because `Ba::method`'s `x` parameter has a default value of 
> > > > > > > `1`); and it returns something of type `Ba*` (because that's what 
> > > > > > > `Ba::method` returns); and it is not considered to be noexcept 
> > > > > > > (because `Ba::method` isn't marked noexcept); and it's okay to 
> > > > > > > discard the result (because `Ba::method` is not nodiscard) and it 
> > > > > > > is tail-called (because `Ba::method` doesn't disallow tail 
> > > > > > > calls). All of these attributes and properties are based on the 
> > > > > > > //static// type of variable `ba`, despite the fact that //at 
> > > > > > > runtime// we'll end up jumping to the code for `Da::method`. 
> > > > > > > According to the source code, statically, `Da::method` has a 
> > > > > > > default argument of `2`, returns `Da*`, is noexcept, and is 
> > > > > > > nodiscard, and disallows tail-calls. But we're not calling 
> > > > > > > `da.method()`, we're calling `ba.method()`; so none of that 
> > > > > > > matters to our call site at `callera`.
> > > > > > > 
> > > > > > > I think this patch is a good thing.
> > > > > > OK, I see. I think this patch is fine then.
> > > > > > 
> > > > > > Should we add an explanation of how virtual functions are handled? 
> > > > > > The doc currently just says the attribute prevents tail-call 
> > > > > > optimization on statically bound calls.
> > > > > > I think this patch is a good thing.
> > > > > 
> > > > > I agree with your explanation but I'm not certain I agree with your 
> > > > > conclusion. :-) I think the examples you point out are more often a 
> > > > > source of confusion for users than not because of the nature of 
> > > > > static vs dynamic dispatch, and so saying "this behavior can be 
> > > > > consistent with all of these other things people often get confused 
> > > > > by" may be justifiable but also seems a bit user-hostile.
> > > > > 
> > > > > Taking a slightly different example:
> > > > > ```
> > > > > struct Ba {
> > > > >   [[clang::not_tail_called]] virtual Ba *method();  
> > > > > };
> > > > > struct Da : Ba {
> > > > >   Ba *method() override; 
> > > > > };
> > > > > 
> > > > > void callera(Da& da) {
> > > > > Ba& ba = da;
> > > > > ba.method();
> > > > > }
> > > > > ```
> > > > > There's no guarantee that `Ba::method()` and `Da::method()` have the 
> > > > > same not-tail-called properties. The codegen for this case will still 
> > > > > attach `notail` to the call site even though the dynamic target may 
> > > > > not meet that requirement.
> > > > > 
> > > > > tl;dr: I think `notail` is a property of the call expression and the 
> > > > > only way to know that's valid is when you know what's being called, 
> > > > > so the current behavior is more user-friendly for avoiding 
> > > > > optimization issues. I'd prefer not to relax that unless there was a 
> > > > > significantly motivating example beyond what's presented here so far.
> > > > > saying "this behavior can be consistent with all of these other 
> > > > > 

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

[Harmen Stoppels via Phabricator via cfe-commits]

haampie added a comment.

I don't have commit access, would be great if you could do that for me!


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

https://reviews.llvm.org/D95119

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

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

In D95119#2584709 , @haampie wrote:

> Should this be merged?

Do you have commit access? If not I can land this for you.


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

https://reviews.llvm.org/D95119

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

[PATCH] D97123: [clangd] Support FixIts that use InsertFromRange instead of inserting raw text

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.cpp:555
+auto Insert = Lexer::getSourceText(FixIt.InsertFromRange, M, L, );
+if (!Invalid)
+  Result.newText = Insert.str();

njames93 wrote:
> kadircet wrote:
> > njames93 wrote:
> > > kadircet wrote:
> > > > it feels like correct semantics would be to make this function fail 
> > > > now. as the resulting value will likely be used for editing the source 
> > > > code and we don't want to propagate some garbage.
> > > While I agree with this in principal. We currently don't handle the case 
> > > where RemoveRange doesn't correspond to a FileCharRange which would 
> > > likely need propagating. Though likely in a separate patch. 
> > i discussed the situation with sam offline (as you've also noticed most of 
> > the sourcelocation -> lsp conversations within this file doesn't really 
> > check for errors), and it looks like this was intentional. we are trying to 
> > cover as much ground as possible in diagnostics.cpp, especially in 
> > https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Diagnostics.cpp#L677.
> >  we should do the same for `InsertFromRange`. all the macroarg logic isn't 
> > necessary (until we notice it is), so just bailing out when the 
> > `InsertFromRange` is a macroid or outside mainfile in `AddFix` should be 
> > enough.
> > 
> > as for testing it, looks like you can invoke a fixit with `InsertFromRange` 
> > via:
> > ```
> > struct Foo {};
> > struct Bar : public [[noreturn]] Foo {};
> > ```
> > 
> > it would be nice to also check with an attribute coming from a macro 
> > expansions.
> Can you elaborate on why we should disregard `InsertFromRange` if it points 
> outside the main file. That seems like an unnecessary restriction. It's 
> probably also safe if its a macro ID, though there's more likely to be some 
> edge case there.
> 
> As for the diagnostic. I don't think there's any value testing that. The test 
> in `SourceCodeTests` covers inserting from range pretty well. It would be 
> nice to test the synthetic messages, but I don't think any diagnostic in 
> clang/clang-tidy contains just 1 fix-it that's an InsertFromRange.
> Can you elaborate on why we should disregard InsertFromRange if it points 
> outside the main file. That seems like an unnecessary restriction.

because we build preamble and main file separately, the sourcemanager is likely 
missing files apart from the main file. hence when one tries to access a 
sourcelocation in them, the file will be re-read from disk and if contents on 
disk change for some reason, clangd might crash when the offsets are off.

>  It's probably also safe if its a macro ID, though there's more likely to be 
> some edge case there.

safeness is a little wrinkly here due to the edge cases (that are hard to 
predict, until we see them in practice) you point out as well. but moreover, do 
we really want to introduce a piece of text coming from a macro expansion into 
source code? is there any use cases for that? (i'd say it should be the macro 
name that should be inserted, not its expansion.)

> As for the diagnostic. I don't think there's any value testing that. 

i wasn't suggesting to introduce a diagnostic test to check the behaviour of 
`toTextEdit`. it was for testing the new logic we'll introduce into 
diagnostics.cpp, sorry if i wasn't clear:/. the tests you currently have are 
totally fine for testing `toTextEdit`.



Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:805
 
+Expected sourceRangeInMainFile(SourceManager , Range R) {
+  if (auto Beg = sourceLocationInMainFile(SM, R.start)) {

sorry if i was vague in preivous comment, i was suggesting making this function 
return a SourceRange all the time by wrapping `sourceLocationInMainFile`s with 
`cantFail`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97123

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

[PATCH] D96976: [analyzer] Fix reinterpret_cast handling for pointer-to-member

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

@steakhal, could you please review this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96976

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

[PATCH] D92195: [OPENMP50]Mapping of the subcomponents with the 'default' mappers.

[Alexey Bataev via Phabricator via cfe-commits]

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

Fixes and updates


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92195

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  clang/test/OpenMP/target_map_codegen_34.cpp
  openmp/libomptarget/src/omptarget.cpp
  openmp/libomptarget/src/private.h
  openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers.cpp

Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers.cpp
@@ -0,0 +1,63 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  int a;
+  double *b;
+} C1;
+#pragma omp declare mapper(C1 s) map(to : s.a) map(from : s.b [0:2])
+
+typedef struct {
+  int a;
+  double *b;
+  C1 c;
+} C;
+#pragma omp declare mapper(C s) map(to : s.a, s.c) map(from : s.b [0:2])
+
+typedef struct {
+  int e;
+  C f;
+  int h;
+} D;
+
+int main() {
+  constexpr int N = 10;
+  D s;
+  s.e = 111;
+  s.f.a = 222;
+  s.f.c.a = 777;
+  double x[2];
+  double x1[2];
+  x[1] = 20;
+  s.f.b = [0];
+  s.f.c.b = [0];
+  s.h = N;
+
+  D *sp = 
+  D **spp = 
+
+  printf("%d %d %d %4.5f %d\n", spp[0][0].e, spp[0][0].f.a, spp[0][0].f.c.a,
+ spp[0][0].f.b[1], spp[0][0].f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+
+  __intptr_t p = reinterpret_cast<__intptr_t>([0]);
+#pragma omp target map(tofrom : spp[0][0]) firstprivate(p)
+  {
+printf("%d %d %d\n", spp[0][0].f.a, spp[0][0].f.c.a,
+   spp[0][0].f.b == reinterpret_cast(p) ? 1 : 0);
+// CHECK: 222 777 0
+spp[0][0].e = 333;
+spp[0][0].f.a = 444;
+spp[0][0].f.c.a = 555;
+spp[0][0].f.b[1] = 40;
+  }
+  printf("%d %d %d %4.5f %d\n", spp[0][0].e, spp[0][0].f.a, spp[0][0].f.c.a,
+ spp[0][0].f.b[1], spp[0][0].f.b == [0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1
+}
Index: openmp/libomptarget/src/private.h
===
--- openmp/libomptarget/src/private.h
+++ openmp/libomptarget/src/private.h
@@ -23,17 +23,20 @@
 extern int targetDataBegin(ident_t *loc, DeviceTy , int32_t arg_num,
void **args_base, void **args, int64_t *arg_sizes,
int64_t *arg_types, map_var_info_t *arg_names,
-   void **arg_mappers, AsyncInfoTy );
+   void **arg_mappers, AsyncInfoTy ,
+   bool FromMapper = false);
 
 extern int targetDataEnd(ident_t *loc, DeviceTy , int32_t ArgNum,
  void **ArgBases, void **Args, int64_t *ArgSizes,
  int64_t *ArgTypes, map_var_info_t *arg_names,
- void **ArgMappers, AsyncInfoTy );
+ void **ArgMappers, AsyncInfoTy ,
+ bool FromMapper = false);
 
 extern int targetDataUpdate(ident_t *loc, DeviceTy , int32_t arg_num,
 void **args_base, void **args, int64_t *arg_sizes,
 int64_t *arg_types, map_var_info_t *arg_names,
-void **arg_mappers, AsyncInfoTy );
+void **arg_mappers, AsyncInfoTy ,
+bool FromMapper = false);
 
 extern int target(ident_t *loc, DeviceTy , void *HostPtr, int32_t ArgNum,
   void **ArgBases, void **Args, int64_t *ArgSizes,
@@ -76,7 +79,8 @@
 // targetDataEnd and targetDataUpdate).
 typedef int (*TargetDataFuncPtrTy)(ident_t *, DeviceTy &, int32_t, void **,
void **, int64_t *, int64_t *,
-   map_var_info_t *, void **, AsyncInfoTy &);
+   map_var_info_t *, void **, AsyncInfoTy &,
+   bool);
 
 // Implemented in libomp, they are called from within __tgt_* functions.
 #ifdef __cplusplus
Index: openmp/libomptarget/src/omptarget.cpp
===
--- openmp/libomptarget/src/omptarget.cpp
+++ openmp/libomptarget/src/omptarget.cpp
@@ -269,10 +269,11 @@
 MapperArgNames[I] = C.Name;
   }
 
-  int rc = target_data_function(
-  loc, Device, MapperComponents.Components.size(), MapperArgsBase.data(),
-  MapperArgs.data(), MapperArgSizes.data(), MapperArgTypes.data(),
-  

[PATCH] D97340: [HIP] Support Spack packages

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:25-31
+// Look for sub-directory starts with Prefix under Path. If there is one and
+// only one matching sub-directory found, append the sub-directory to Path. If
+// there is no matching sub-directory or there are more than one matching
+// sub-directories, diagnose them. Returns true if there is only one matching
+// sub-directory.
+static void handleSPACKPackage(const Driver , SmallString<0> ,
+   StringRef Prefix) {

`void` function can not return `true`, so the comment needs updating.

Also, Using Path as both an input and an output is odd. We should probably 
return the full path or an empty string and call the function 
`findSPACKPackage`.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:27-28
+// only one matching sub-directory found, append the sub-directory to Path. If
+// there is no matching sub-directory or there are more than one matching
+// sub-directories, diagnose them. Returns true if there is only one matching
+// sub-directory.

How are users expected to handle cases when they do have multiple rocm versions 
installed with spack (I assume that having multiple packages  *is* possible. 
E.g. package-4.0, package-3.7 should be able to co-exist). They may have 
legitimate reasons to have more than one rocm version installed *and* they may 
need to be able to build with a particular one.

Can users use `--hip-path=specific/hip/version/in-spack` ?



Comment at: clang/lib/Driver/ToolChains/ROCm.h:56
+// convention --.
+llvm::SmallString<0> SPACKReleaseStr;
+  };

Nit: using `SmallString` here and everywhere else in this patch does not buy us 
anything. `std::string` would be fine.
My own rule of thumb is to use standard types by default and optimized types 
when they are needed. 


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

https://reviews.llvm.org/D97340

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

[PATCH] D97400: [clang][NFC] Remove unnecessary string copies in CustomDiagInfo

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: rsmith, aaron.ballman.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Lookup the map using a string ref and store indexed diag info using a StringRef 
to the map.
It may be worth using DenseMap to store this, but I'll leave that option on the 
table for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97400

Files:
  clang/lib/Basic/DiagnosticIDs.cpp


Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -356,42 +356,56 @@
 
 namespace clang {
   namespace diag {
-class CustomDiagInfo {
-  typedef std::pair DiagDesc;
-  std::vector DiagInfo;
-  std::map DiagIDs;
-public:
-
-  /// getDescription - Return the description of the specified custom
-  /// diagnostic.
-  StringRef getDescription(unsigned DiagID) const {
-assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
-   "Invalid diagnostic ID");
-return DiagInfo[DiagID-DIAG_UPPER_LIMIT].second;
-  }
-
-  /// getLevel - Return the level of the specified custom diagnostic.
-  DiagnosticIDs::Level getLevel(unsigned DiagID) const {
-assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
-   "Invalid diagnostic ID");
-return DiagInfo[DiagID-DIAG_UPPER_LIMIT].first;
-  }
-
-  unsigned getOrCreateDiagID(DiagnosticIDs::Level L, StringRef Message,
- DiagnosticIDs ) {
-DiagDesc D(L, std::string(Message));
-// Check to see if it already exists.
-std::map::iterator I = DiagIDs.lower_bound(D);
-if (I != DiagIDs.end() && I->first == D)
-  return I->second;
-
-// If not, assign a new ID.
-unsigned ID = DiagInfo.size()+DIAG_UPPER_LIMIT;
-DiagIDs.insert(std::make_pair(D, ID));
-DiagInfo.push_back(D);
-return ID;
-  }
-};
+  using DiagDesc = std::pair;
+  struct DiagDescRef {
+DiagDescRef(DiagnosticIDs::Level Level, StringRef FormatString)
+: Level(Level), FormatString(FormatString) {}
+DiagDescRef(const DiagDesc ) : DiagDescRef(D.first, D.second) {}
+DiagnosticIDs::Level Level;
+StringRef FormatString;
+  };
+  static bool operator<(const DiagDesc , const DiagDescRef ) {
+if (LHS.first == RHS.Level)
+  return LHS.second < RHS.FormatString;
+return LHS.first < RHS.Level;
+  }
+  class CustomDiagInfo {
+std::vector DiagInfo;
+std::map> DiagIDs;
+
+  public:
+/// getDescription - Return the description of the specified custom
+/// diagnostic.
+StringRef getDescription(unsigned DiagID) const {
+  assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
+ "Invalid diagnostic ID");
+  return DiagInfo[DiagID - DIAG_UPPER_LIMIT].FormatString;
+}
+
+/// getLevel - Return the level of the specified custom diagnostic.
+DiagnosticIDs::Level getLevel(unsigned DiagID) const {
+  assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
+ "Invalid diagnostic ID");
+  return DiagInfo[DiagID - DIAG_UPPER_LIMIT].Level;
+}
+
+unsigned getOrCreateDiagID(DiagnosticIDs::Level L, StringRef Message,
+   DiagnosticIDs ) {
+  // Check to see if it already exists.
+
+  auto I = DiagIDs.lower_bound(DiagDescRef(L, Message));
+  if (I != DiagIDs.end() && I->first.first == L &&
+  I->first.second == Message)
+return I->second;
+
+  // If not, assign a new ID.
+  unsigned ID = DiagInfo.size() + DIAG_UPPER_LIMIT;
+  auto InsertRet =
+  DiagIDs.insert(std::make_pair(DiagDesc{L, Message.str()}, ID));
+  DiagInfo.emplace_back(InsertRet.first->first);
+  return ID;
+}
+  };
 
   } // end diag namespace
 } // end clang namespace


Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -356,42 +356,56 @@
 
 namespace clang {
   namespace diag {
-class CustomDiagInfo {
-  typedef std::pair DiagDesc;
-  std::vector DiagInfo;
-  std::map DiagIDs;
-public:
-
-  /// getDescription - Return the description of the specified custom
-  /// diagnostic.
-  StringRef getDescription(unsigned DiagID) const {
-assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
-   "Invalid diagnostic ID");
-return DiagInfo[DiagID-DIAG_UPPER_LIMIT].second;
-  }
-
-  /// getLevel - Return the level of the specified custom diagnostic.
-  DiagnosticIDs::Level getLevel(unsigned DiagID) const {
-assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
-   "Invalid diagnostic ID");
-return 

[PATCH] D96847: [clang][cli] Store additional optimization remarks info

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96847

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

[PATCH] D97327: [NFC] Switch to auto marshalling infrastructure for `-fsanitize-address-destructor-kind=` flag.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 accepted this revision.
jansvoboda11 added a comment.

LGTM.

If you don't get around committing this today, please rebase this on top of 
D97375  that I'm going to commit tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97327

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

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

> @jansvoboda11 Thanks. If it's okay with you I'll make the change to use the 
> new marshalling infrastructure in a separate patch 
> (https://reviews.llvm.org/D97327) because I may need to back port this patch 
> to an older LLVM branch.

That's fine by me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96572

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

[PATCH] D97226: [clangd] Show hex value of numeric constants

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.

still lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97226

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

[PATCH] D97233: Support `#pragma clang section` directives on MachO targets

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 326126.
jroelofs added a comment.

Mark section invalid if the target doesn't like how it's spelled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97233

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCXX/clang-sections.cpp
  clang/test/Sema/pragma-clang-section-macho.c
  llvm/include/llvm/MC/MCSectionMachO.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/MC/MCParser/DarwinAsmParser.cpp
  llvm/lib/MC/MCSectionMachO.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/CodeGen/AArch64/clang-section-macho.ll

Index: llvm/test/CodeGen/AArch64/clang-section-macho.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/clang-section-macho.ll
@@ -0,0 +1,11 @@
+;RUN: llc -mtriple=arm64-apple-ios %s -o - | FileCheck %s
+
+define dso_local void @foo() #0 {
+entry:
+  ret void
+}
+
+attributes #0 = { "implicit-section-name"="__TEXT,__mytext" }
+
+; CHECK:  .section	__TEXT,__mytext
+; CHECK-NEXT: .globl	_foo
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1899,9 +1899,8 @@
   StringRef ParsedSegment, ParsedSection;
   unsigned TAA = 0, StubSize = 0;
   bool TAAParsed;
-  std::string ErrorCode = MCSectionMachO::ParseSectionSpecifier(
-  Section, ParsedSegment, ParsedSection, TAA, TAAParsed, StubSize);
-  assert(ErrorCode.empty() && "Invalid section specifier.");
+  cantFail(MCSectionMachO::ParseSectionSpecifier(
+  Section, ParsedSegment, ParsedSection, TAA, TAAParsed, StubSize));
 
   // Ignore the globals from the __OBJC section. The ObjC runtime assumes
   // those conform to /usr/lib/objc/runtime.h, so we can't add redzones to
Index: llvm/lib/MC/MCSectionMachO.cpp
===
--- llvm/lib/MC/MCSectionMachO.cpp
+++ llvm/lib/MC/MCSectionMachO.cpp
@@ -174,12 +174,12 @@
 /// flavored .s file.  If successful, this fills in the specified Out
 /// parameters and returns an empty string.  When an invalid section
 /// specifier is present, this returns a string indicating the problem.
-std::string MCSectionMachO::ParseSectionSpecifier(StringRef Spec,// In.
-  StringRef ,// Out.
-  StringRef ,// Out.
-  unsigned  ,// Out.
-  bool  ,  // Out.
-  unsigned  ) { // Out.
+Error MCSectionMachO::ParseSectionSpecifier(StringRef Spec,   // In.
+StringRef ,   // Out.
+StringRef ,   // Out.
+unsigned ,// Out.
+bool ,  // Out.
+unsigned ) { // Out.
   TAAParsed = false;
 
   SmallVector SplitSpec;
@@ -194,25 +194,23 @@
   StringRef Attrs = GetEmptyOrTrim(3);
   StringRef StubSizeStr = GetEmptyOrTrim(4);
 
-  // Verify that the segment is present and not too long.
-  if (Segment.empty() || Segment.size() > 16)
-return "mach-o section specifier requires a segment whose length is "
-   "between 1 and 16 characters";
-
-  // Verify that the section is present and not too long.
+  // Verify that the section is present.
   if (Section.empty())
-return "mach-o section specifier requires a segment and section "
-   "separated by a comma";
+return createStringError(inconvertibleErrorCode(),
+ "mach-o section specifier requires a segment "
+ "and section separated by a comma");
 
+  // Verify that the section is not too long.
   if (Section.size() > 16)
-return "mach-o section specifier requires a section whose length is "
-   "between 1 and 16 characters";
+return createStringError(inconvertibleErrorCode(),
+ "mach-o section specifier requires a section "
+ "whose length is between 1 and 16 characters");
 
   // If there is no comma after the section, we're done.
   TAA = 0;
   StubSize = 0;
   if (SectionType.empty())
-return "";
+return Error::success();
 
   // Figure out which section type it is.
   auto TypeDescriptor =
@@ -223,7 +221,9 @@
 
   // If we didn't find the section type, 

[PATCH] D97072: [OpenCL][Docs] Add guidelines for adding new extensions and features

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: clang/docs/OpenCLSupport.rst:196
+If an extension adds the functionality that does not modify standard language
+parsing it should not require clang source code modifications. Most commonly
+such extensions add functionality via libraries (by adding new non-native

Anastasia wrote:
> svenvh wrote:
> > This might need some rephrasing to take TableGen BIFs into account.  I 
> > would consider `clang/lib/Sema/OpenCLBuiltins.td` an integral part of the 
> > clang source code.
> > 
> > Maybe rephrase "clang source code modifications" into "clang parser source 
> > code modifications"?
> I am trying to avoid using parsing in relation to clang since someone might 
> understand it as clang `Parser` library functionality. I have added a note 
> about this case explicitly
> 
> > Note that new functionality in ``OpenCLBuiltins.td`` detailed in 
> > :ref:`` belong to this category even if it is part of the 
> > clang source code.
> 
> Although we do refer to header functionality in the next paragraph anyway 
> where this is referenced but indirectly. I hope it provides enough 
> clarifications for now.
This makes it very confusing for the reader, because you're referring to "this 
category" without defining what a category is.

How about removing that note, and appending to the line above instead:

If an extension adds functionality that does not modify standard language 
parsing it should not require //modifying anything other than header files and 
``OpenCLBuiltins.td`` as detailed in :ref:``//



Comment at: clang/docs/OpenCLSupport.rst:201
+
+Clang has standard headers where new types and functions are being added,
+for more details refer to

Anastasia wrote:
> svenvh wrote:
> > Should this mention `clang/lib/Sema/OpenCLBuiltins.td` too?
> I think here I don't want to go to too many details of how headers are 
> implemented. I refer to this: 
> https://clang.llvm.org/docs/UsersManual.html#opencl-header that also refers 
> to `OpenCLSupport` page where we explain details on Tablegen header. However, 
> the header topic does require another round of clarifications and I do plan 
> to improve it further iin the near future. 
Fair enough; with the addition I'm suggesting above my concern would be 
addressed anyway.



Comment at: clang/docs/OpenCLSupport.rst:219
+
+Pragmas without detailed information of its behavior (explanation of changes it
+triggers in the parsing) should not be added to clang. Moreover, the acceptable

mantognini wrote:
> svenvh wrote:
> > 
> "of //their// behavior", I think.
Yes!



Comment at: clang/docs/OpenCLSupport.rst:221
+triggers in the parsing) should not be added to clang. Moreover, the acceptable
+behavior of pragmas should provide useful functionality to the user.
+

Anastasia wrote:
> svenvh wrote:
> > Remove "acceptable behavior of".
> > 
> > How do you define "useful functionality"?
> I think this is one of those situations that is hard to define unambiguously, 
> so I am open to suggestions. However, I do want to prevent changes that are 
> reinventing the wheel (i.e. C/C++ already had another way to do the same 
> thing) or functionality that doesn't add any value (i.e. requiring pragma 
> enable to use already added types or functions). This has happened in the 
> past multiple times so I think it is good to be specific that when new 
> functionality is added it should have a reason for it.
> 
> I think the other statement above:
> 
> > New functionality is accepted as soon as the documentation is detailed to 
> > the level sufficient to be implemented.
> 
> is similar. It is not very easy to tell what is the detailed level of 
> documentation. FYI it generally aligns with clang overall policy:
> 
> https://clang.llvm.org/get_involved.html
> 
> > A specification: The specification must be sufficient to understand the 
> > design of the feature as well as interpret the meaning of specific 
> > examples. The specification should be detailed enough that another compiler 
> > vendor could implement the feature.
> 
> I am just emphasizing this item here.
> 
> Regarding 'usefulness' I would even suggest adding it as a general guideline 
> for clang but I think this is not very common. So for now I want to make sure 
> we have it in our guidelines at least since we have encountered this.
I see, though I'm a bit hesitant to accept the word "useful" without any 
further explanation. Would adding the following make sense?

... should provide useful functionality //that cannot be achieved by other 
means// to the user.


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

https://reviews.llvm.org/D97072

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

[PATCH] D97273: OpenMP: Fix object clobbering issue when using save-temps

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Works everywhere we have tried it. Fundamentally it renames a temporary file, 
so shouldn't break much. Will be great to have -save-temps working for nvptx.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97273

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

[PATCH] D97273: OpenMP: Fix object clobbering issue when using save-temps

[Johannes Doerfert via Phabricator via cfe-commits]

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

LGTM, assuming it doesn't break support the reasoning makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97273

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

[PATCH] D96838: Add GNU attribute 'retain'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

We have two implementation choices.

1. Lift the source language restriction (it is currently discouraged) on 
llvm.compiler.used, let llvm.used use `SHF_GNU_RETAIN` on ELF, and change 
`__attribute__((used))` to use llvm.compiler.used on ELF.
2. Don't touch the backend semantics of llvm.used or llvm.compiler.used. Add a 
metadata `!retain` and let `__attribute__((retain))` use that. This is what has 
been implemented in D96837  and this patch.

I posted https://lists.llvm.org/pipermail/llvm-dev/2021-February/148760.html 
(llvm-dev & cfe-dev) (my first message mentions that I lean to 1. I actually 
lean to 2.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:192
   CC1Args.push_back("-fcuda-is-device");
-  CC1Args.push_back("-emit-llvm-bc");
 

vaguely interesting that `-emit-llvm` here appears to generate the same code as 
`-emit-llvm-bc`, though neither compose correctly for `-save-temps`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96769

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

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Replacing `CC1Args.push_back("-emit-llvm-bc");` with 
`CC1Args.push_back("-emit-llvm-bc");` as suggested on the call does not work. 
This hook is downstream of the clang driver, so all it does under save temps is 
lead to `clang -E -emit-llvm`, which generated llvm as requested, that cannot 
be fed into `clang -x cpp-output`.

The handling of `clang -save-temps` that strips out `emit-llvm` from the 
preprocessor pass runs before this.

I do not want to change the semantics of emit-llvm, emit-llvm-bc, or save-temps 
to help out the amdgcn openmp target. I can see that breaking lots of other 
users of clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96769

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

[PATCH] D96838: Add GNU attribute 'retain'

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Aha; attribute `used` *by itself* is not sufficient to preserve sections in the 
output.  But the `__start_/__stop_` symbols implicitly create a reference to 
each of the named sections, and that implicit reference can preserve them in 
the output (assuming gc roots etc).
So, the idea is that attribute `retain` can be used *instead* of the 
`__start_/__stop_` symbols, to preserve sections in the output (with the 
advantage that it will work even for sections that do not have a C-identifier 
name).

Thanks for helping me understand this from a user perspective.  That will be 
important when you go to write the release note for this new attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D97072: [OpenCL][Docs] Add guidelines for adding new extensions and features

[Marco Antognini via Phabricator via cfe-commits]

mantognini added a comment.

Some minor comments.




Comment at: clang/docs/OpenCLSupport.rst:197
+parsing it should not require clang source code modifications. Most commonly
+such extensions add functionality via libraries (by adding new non-native
+types or functions) parsed regularly. Similar to other languages this is the





Comment at: clang/docs/OpenCLSupport.rst:219
+
+Pragmas without detailed information of its behavior (explanation of changes it
+triggers in the parsing) should not be added to clang. Moreover, the acceptable

svenvh wrote:
> 
"of //their// behavior", I think.



Comment at: clang/docs/OpenCLSupport.rst:223
+
+Note that some legacy extensions (published prior to OpenCL 3.0), however still
+provide some non-conformant functionality for pragmas e.g. add diagnostics on




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

https://reviews.llvm.org/D97072

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

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen updated this revision to Diff 326107.
khchen marked 2 inline comments as done.
khchen added a comment.

address https://reviews.llvm.org/D95016?id=324197#inline-912573


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/riscv_vector.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/test/CodeGen/RISCV/rvv-intrinsics-generic/vadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-generic/vfadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfadd.c
  clang/test/CodeGen/RISCV/vadd.c
  clang/test/Headers/riscv-vector-header.c
  clang/utils/TableGen/CMakeLists.txt
  clang/utils/TableGen/RISCVVEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h
  llvm/docs/CommandGuide/tblgen.rst

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

[PATCH] D97233: Support `#pragma clang section` directives on MachO targets

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:304
 
+  if (llvm::Error E = Context.getTargetInfo().isValidSectionSpecifier(SecName))
+Diag(PragmaLoc, diag::err_pragma_section_invalid_for_target)

t.p.northover wrote:
> Shouldn't this block return so the section doesn't get marked as valid later?
yes!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97233

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth added a comment.

The option is used in the wild, but not as widely as I first believed.  We've 
already fixed a couple projects that were using it.  I think the Release Note 
is the right solution.  Thanks for doing that.  I withdraw my suggestion to 
allow and ignore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D97361: [clang-tidy] Add misc-redundant-using check

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:23
+  Finder->addMatcher(
+  usingDecl(isExpansionInMainFile()).bind("using-declaration"), this);
+  Finder->addMatcher(

Why restrict these to main file only?



Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:30-33
+  if (const auto *Declaration =
+  Result.Nodes.getNodeAs("using-declaration")) {
+checkUsingDecl(Declaration, Result);
+  }

nit: Elide braces of single line if statements.



Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:35-38
+  if (const auto *Directive =
+  Result.Nodes.getNodeAs("using-directive")) {
+checkUsingDirective(Directive, Result);
+  }





Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:45
+
+  for (const auto *UsingShadow : Declaration->shadows()) {
+const Decl *TargetDecl = UsingShadow->getTargetDecl()->getCanonicalDecl();

nit: don't use auto as type isn't explicitly spelled in the initialization.



Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:50
+  const auto *NSD = cast(TargetDecl->getDeclContext());
+  diagUsing("declaration", Declaration, Declaration, NSD, Result);
+}

Shouldn't there be a break after we have diagnosed this?



Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:78
+  diag(Using->getLocation(),
+   "using %0 %1 is redundant, already in namespace %2")
+  << Type << ND << NSD;

nit: The canonical way to do diagnostics like this is with select. Then pass 0 
for declaration and 1 for directive.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97361

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

[PATCH] D97364: [docs] Add a release note for the removing of -Wreturn-std-move-in-c++11

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth accepted this revision.
amccarth 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/D97364/new/

https://reviews.llvm.org/D97364

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

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen added inline comments.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:899
+// (operand) in ProtoSeq. ProtoSeq[0] is output operand.
+SmallVector ProtoSeq;
+const StringRef Primaries("evwqom0ztc");

craig.topper wrote:
> I think this is something like
> 
> ```
> while (!Prototypes.empty()) {
>  auto Idx = Prototypes.find_first_of(Primaries);
>  assert(Idx != StringRef::npos);
>  ProtoSeq.push_back(Prototypes.slice(0, Idx+1).str());
>  Prototypes = Prototypes.drop_front(Idx+1);
> }
> ```
> 
> Which might be easier to understand.
Thanks, it's clear.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1016
+  SmallVector ExtVector;
+  // D implies F
+  if (Extents & RISCV_Extension::F) {

craig.topper wrote:
> I don't understand this. It says D implies F but we're checking for F. So 
> that seems like F implies D.
Remove extension implying rule. 
I thought we don't need to consider implying rule because it's clang's 
responsibility. 
The original thinking was when predecessor "only" defines `__riscv_d`, and 
floating instruction also need to supported. But in fact,  when enabling 
__riscv_d predecessor will also define `__riscv_f`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016

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