[clang] a6a15dd - [Driver] Delete toplevel i386-gnu/gcc detection in favor of i386-gnu alias triple detection

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-03-19T22:50:36-07:00
New Revision: a6a15dde5a870f0ce6be0ea26d36cec60e846a2d

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

LOG: [Driver] Delete toplevel i386-gnu/gcc detection in favor of i386-gnu alias 
triple detection

This is used by hurd.c (usr/lib/gcc/i386-gnu/4.6.0) but we can leverage
the existing alias triple detection.

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 906bac57fa77..3c1fc87d7896 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2537,9 +2537,6 @@ void 
Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
   // FIXME: It may be worthwhile to generalize this and look for a second
   // triple.
   {"i386-linux-gnu/gcc/" + CandidateTriple.str(), "../../..",
-   (TargetArch == llvm::Triple::x86 &&
-TargetTriple.getOS() != llvm::Triple::Solaris)},
-  {"i386-gnu/gcc/" + CandidateTriple.str(), "../../..",
(TargetArch == llvm::Triple::x86 &&
 TargetTriple.getOS() != llvm::Triple::Solaris)}};
 



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

[clang] 1f4959b - [Driver] Drop unneeded $triple/gcc/$triple detection

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-03-19T22:45:50-07:00
New Revision: 1f4959b27607d4748c83820ffcf8bf24f09fdd47

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

LOG: [Driver] Drop unneeded $triple/gcc/$triple detection

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index eb32f4b920b5..906bac57fa77 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2532,12 +2532,6 @@ void 
Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
TargetTriple.getVendor() == llvm::Triple::Freescale ||
TargetTriple.getVendor() == llvm::Triple::OpenEmbedded},
 
-  // Natively multiarch systems sometimes put the GCC triple-specific
-  // directory within their multiarch lib directory, resulting in the
-  // triple appearing twice.
-  {CandidateTriple.str() + "/gcc/" + CandidateTriple.str(), "../../..",
-   TargetTriple.getOS() != llvm::Triple::Solaris},
-
   // Deal with cases (on Ubuntu) where the system architecture could be 
i386
   // but the GCC target architecture could be (say) i686.
   // FIXME: It may be worthwhile to generalize this and look for a second



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

[PATCH] D99009: [RISCV] [1/2] Add intrinsic for Zbr extension

[LevyHsu via Phabricator via cfe-commits]

LevyHsu created this revision.
LevyHsu added reviewers: craig.topper, kito-cheng, evandro, khchen.
LevyHsu added projects: clang, LLVM.
Herald added subscribers: vkmr, frasercrmck, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, 
niosHD, sabuasal, simoncook, johnrusso, rbar, asb, hiraditya.
LevyHsu requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.

Implementation for RISC-V Zbr extension intrinsics.

head file is included in the second patch incase the name needs to be changed

RV32 / 64:

  crc32b
  crc32h
  crc32w
  crc32cb
  crc32ch
  crc32cw

RV64 Only:

  crc32d
  crc32cd


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99009

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbr.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbr.c
  clang/test/Headers/rvintrin.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/lib/Target/RISCV/RISCVInstrInfoB.td
  llvm/test/CodeGen/RISCV/rv32Zbr.ll
  llvm/test/CodeGen/RISCV/rv64Zbr.ll

Index: llvm/test/CodeGen/RISCV/rv64Zbr.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv64Zbr.ll
@@ -0,0 +1,91 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=experimental-zbr -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64ZBR
+
+declare i64 @llvm.riscv.crc32.b.i64(i64)
+
+define i64 @crc32b(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32b:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32.b a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32.b.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32.h.i64(i64)
+
+define i64 @crc32h(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32h:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32.h a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32.h.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32.w.i64(i64)
+
+define i64 @crc32w(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32w:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32.w a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32.w.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32c.b.i64(i64)
+
+define i64 @crc32cb(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32cb:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32c.b a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32c.b.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32c.h.i64(i64)
+
+define i64 @crc32ch(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32ch:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32c.h a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32c.h.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32c.w.i64(i64)
+
+define i64 @crc32cw(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32cw:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32c.w a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32c.w.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32.d.i64(i64)
+
+define i64 @crc32d(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32d:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32.d a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32.d.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32c.d.i64(i64)
+
+define i64 @crc32cd(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32cd:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32c.d a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32c.d.i64(i64 %a)
+ ret i64 %tmp
+}
Index: llvm/test/CodeGen/RISCV/rv32Zbr.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv32Zbr.ll
@@ -0,0 +1,69 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=experimental-zbr -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32ZBR
+
+declare i32 @llvm.riscv.crc32.b.i32(i32)
+
+define i32 @crc32b(i32 %a) nounwind {
+; RV32ZBR-LABEL: crc32b:
+; RV32ZBR:   # %bb.0:
+; RV32ZBR-NEXT:crc32.b a0, a0
+; RV32ZBR-NEXT:ret
+  %tmp = call i32 @llvm.riscv.crc32.b.i32(i32 %a)
+ ret i32 %tmp
+}
+
+declare i32 @llvm.riscv.crc32.h.i32(i32)
+
+define i32 @crc32h(i32 %a) nounwind {
+; RV32ZBR-LABEL: crc32h:
+; RV32ZBR:   # %bb.0:
+; RV32ZBR-NEXT:crc32.h a0, a0
+; RV32ZBR-NEXT:ret
+  %tmp = call i32 @llvm.riscv.crc32.h.i32(i32 %a)
+ ret i32 %tmp
+}
+
+declare i32 @llvm.riscv.crc32.w.i32(i32)
+
+define i32 @crc32w(i32 %a) nounwind {
+; RV32ZBR-LABEL: crc32w:
+; RV32ZBR:   # %bb.0:
+; RV32ZBR-NEXT:crc32.w a0, a0
+; RV32ZBR-NEXT:ret

[PATCH] D99008: [RISCV] Add intrinsic for Zbr extension

[LevyHsu via Phabricator via cfe-commits]

LevyHsu updated this revision to Diff 332076.
Herald added subscribers: usaxena95, kadircet, arphaman, mgorny.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99008

Files:
  clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/riscv_zbr_intrin.h
  clang/lib/Headers/rvintrin.h

Index: clang/lib/Headers/rvintrin.h
===
--- /dev/null
+++ clang/lib/Headers/rvintrin.h
@@ -0,0 +1,30 @@
+/* === rvintrin.h --===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __RVINTRIN_H
+#define __RVINTRIN_H
+
+// Long is 32 bit on riscv32 and 64 bit on riscv64 according to calling convention.
+#define int_xlen_t long
+#define uint_xlen_t unsigned int_xlen_t
+
+_Static_assert(__riscv_xlen == sizeof(uint_xlen_t) * 8,
+   "uint_xlen_t is not __riscv_xlen bits long");
+
+#define __DEFAULT_FN_ATTRS \
+  __attribute__((__always_inline__, __artificial__, __nodebug__))
+
+#if defined(__riscv_zbr)
+#include "riscv_zbr_intrin.h"
+#endif
+
+#undef __DEFAULT_FN_ATTRS
+#undef uint_xlen_t
+#undef int_xlen_t
+#endif // __RVINTRIN_H
\ No newline at end of file
Index: clang/lib/Headers/riscv_zbr_intrin.h
===
--- /dev/null
+++ clang/lib/Headers/riscv_zbr_intrin.h
@@ -0,0 +1,68 @@
+/* === riscv_zbr_intrin.h --===
+*
+* Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+* See https://llvm.org/LICENSE.txt for license information.
+* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+*
+*===---===
+*/
+
+#ifndef __RISCV_ZBR_INTRIN_H
+#define __RISCV_ZBR_INTRIN_H
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+// Zbr
+static __inline__ int_xlen_t __DEFAULT_FN_ATTRS
+_rv_crc32_b(int_xlen_t rs1) {
+  return __builtin_riscv_crc32_b(rs1);
+}
+
+static __inline__ int_xlen_t __DEFAULT_FN_ATTRS
+_rv_crc32_h(int_xlen_t rs1) {
+  return __builtin_riscv_crc32_h(rs1);
+}
+
+static __inline__ int_xlen_t __DEFAULT_FN_ATTRS
+_rv_crc32_w(int_xlen_t rs1) {
+  return __builtin_riscv_crc32_w(rs1);
+}
+
+static __inline__ int_xlen_t __DEFAULT_FN_ATTRS
+_rv_crc32c_b(int_xlen_t rs1) {
+  return __builtin_riscv_crc32c_b(rs1);
+}
+
+static __inline__ int_xlen_t __DEFAULT_FN_ATTRS
+_rv_crc32c_h(int_xlen_t rs1) {
+  return __builtin_riscv_crc32c_h(rs1);
+}
+
+static __inline__ int_xlen_t __DEFAULT_FN_ATTRS
+_rv_crc32c_w(int_xlen_t rs1) {
+  return __builtin_riscv_crc32c_w(rs1);
+}
+
+// RV64 only intrinsics
+
+#if __riscv_xlen == 64
+
+static __inline__ int_xlen_t __DEFAULT_FN_ATTRS
+_rv_crc32_d(int_xlen_t rs1) {
+return __builtin_riscv_crc32_d(rs1);
+}
+
+static __inline__ int_xlen_t __DEFAULT_FN_ATTRS
+_rv_crc32c_d(int_xlen_t rs1) {
+return __builtin_riscv_crc32c_d(rs1);
+}
+
+#endif // if defined(__riscv64__)
+
+#if defined(__cplusplus)
+}
+#endif // if defined(__cplusplus)
+
+#endif // __RISCV_ZBR_INTRIN_H
\ No newline at end of file
Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -97,6 +97,8 @@
   ptwriteintrin.h
   rdseedintrin.h
   rtmintrin.h
+  rvintrin.h
+  riscv_zbr_intrin.h
   serializeintrin.h
   sgxintrin.h
   s390intrin.h
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -152,6 +152,8 @@
   {"include/prfchwintrin.h", ""},
   {"include/rdseedintrin.h", ""},
   {"include/rtmintrin.h", ""},
+  {"include/rvintrin.h", ""},
+  {"include/riscv_zbr_intrin.h", ""},
   {"include/shaintrin.h", ""},
   {"include/smmintrin.h", ""},
   {"include/stdalign.h", ""},
Index: clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp

[PATCH] D99008: [RISCV] Add intrinsic for Zbr extension

[LevyHsu via Phabricator via cfe-commits]

LevyHsu created this revision.
LevyHsu added reviewers: craig.topper, evandro, kito-cheng, khchen.
LevyHsu added projects: clang, LLVM.
Herald added subscribers: vkmr, frasercrmck, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, 
niosHD, sabuasal, simoncook, johnrusso, rbar, asb, hiraditya.
LevyHsu requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.

Implementation for RISC-V Zbr extension intrinsics.

RV32 / 64:

  crc32b
  crc32h
  crc32w
  crc32cb
  crc32ch
  crc32cw

RV64 Only:

  crc32d
  crc32cd


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99008

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbr.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbr.c
  clang/test/Headers/rvintrin.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/lib/Target/RISCV/RISCVInstrInfoB.td
  llvm/test/CodeGen/RISCV/rv32Zbr.ll
  llvm/test/CodeGen/RISCV/rv64Zbr.ll

Index: llvm/test/CodeGen/RISCV/rv64Zbr.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv64Zbr.ll
@@ -0,0 +1,91 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=experimental-zbr -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64ZBR
+
+declare i64 @llvm.riscv.crc32.b.i64(i64)
+
+define i64 @crc32b(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32b:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32.b a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32.b.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32.h.i64(i64)
+
+define i64 @crc32h(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32h:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32.h a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32.h.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32.w.i64(i64)
+
+define i64 @crc32w(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32w:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32.w a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32.w.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32c.b.i64(i64)
+
+define i64 @crc32cb(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32cb:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32c.b a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32c.b.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32c.h.i64(i64)
+
+define i64 @crc32ch(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32ch:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32c.h a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32c.h.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32c.w.i64(i64)
+
+define i64 @crc32cw(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32cw:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32c.w a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32c.w.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32.d.i64(i64)
+
+define i64 @crc32d(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32d:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32.d a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32.d.i64(i64 %a)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.crc32c.d.i64(i64)
+
+define i64 @crc32cd(i64 %a) nounwind {
+; RV64ZBR-LABEL: crc32cd:
+; RV64ZBR:   # %bb.0:
+; RV64ZBR-NEXT:crc32c.d a0, a0
+; RV64ZBR-NEXT:ret
+  %tmp = call i64 @llvm.riscv.crc32c.d.i64(i64 %a)
+ ret i64 %tmp
+}
Index: llvm/test/CodeGen/RISCV/rv32Zbr.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv32Zbr.ll
@@ -0,0 +1,69 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=experimental-zbr -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32ZBR
+
+declare i32 @llvm.riscv.crc32.b.i32(i32)
+
+define i32 @crc32b(i32 %a) nounwind {
+; RV32ZBR-LABEL: crc32b:
+; RV32ZBR:   # %bb.0:
+; RV32ZBR-NEXT:crc32.b a0, a0
+; RV32ZBR-NEXT:ret
+  %tmp = call i32 @llvm.riscv.crc32.b.i32(i32 %a)
+ ret i32 %tmp
+}
+
+declare i32 @llvm.riscv.crc32.h.i32(i32)
+
+define i32 @crc32h(i32 %a) nounwind {
+; RV32ZBR-LABEL: crc32h:
+; RV32ZBR:   # %bb.0:
+; RV32ZBR-NEXT:crc32.h a0, a0
+; RV32ZBR-NEXT:ret
+  %tmp = call i32 @llvm.riscv.crc32.h.i32(i32 %a)
+ ret i32 %tmp
+}
+
+declare i32 @llvm.riscv.crc32.w.i32(i32)
+
+define i32 @crc32w(i32 %a) nounwind {
+; RV32ZBR-LABEL: crc32w:
+; RV32ZBR:   # %bb.0:
+; RV32ZBR-NEXT:crc32.w a0, a0
+; RV32ZBR-NEXT:ret
+  %tmp = call i32 @llvm.riscv.crc32.w.i32(i32 %a)
+ ret i32 %tmp
+}
+
+declare 

[PATCH] D98783: [CUDA][HIP] Remove unused addr space casts

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D98783#2633261 , @tra wrote:

> In D98783#2632244 , @yaxunl wrote:
>
>> In D98783#2632143 , @tra wrote:
>>
>>> Shouldn't the unused ASCs be eliminated by DCE? We seem to be cleaning up 
>>> the consequences of a problem that happened somewhere else.
>>
>> DCE does not eliminate these unused ASCs because normally they should not 
>> exist. That's why we do not want them to be kept in the IR emitted by clang.
>
> TBH, these 'invisible' ASCs do bother me. I wonder what else is present in 
> the IR that we can't easily examine with existing tools?
> In principle I'm OK with eliminating such dangling ASCs here as a short-term 
> workaround, but I think it's potentially a more general issue which needs a 
> more general solution.

The invisible LLVM constants only happen when they are created but not used 
later, which is rare since usually a constant is created and used immediately, 
making them visible.

> One way to consistently deal with something like that is to codegen something 
> using those ASCs, but which would be considered unused and later DCE'd along 
> with unused ASCs. 
> We could use some sort of counterpart for `@llvm.used` only for potentially 
> unused IR we create.
> Tying them to such `@llvm_maybe_unused` would avoid the problem.

That's one solution. Another solution is to add a member function to LLVM 
module to clean up unused LLVM constants and add check to LLVM IR verifier to 
make sure there is no unused constants.


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

https://reviews.llvm.org/D98783

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

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1754
+
+  int RC = llvm::sys::ExecuteAndWait(GCCPath, GCCArgs, None, Redirects);
+

Even if ExecuteAndWait is accepted, you may consider a pipe. The current 
implementation leaves temporary files under /tmp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97916

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

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay requested changes to this revision.
MaskRay added a comment.
This revision now requires changes to proceed.

In D97916#2625170 , @luismarques wrote:

> This patch seems to be in pretty good shape now.
>
> One thing that might be useful (important?) to add is additional diagnostics 
> when run in verbose mode. Currently `clang -v` will indicate that it found 
> the GCC installation and will list the multilibs but there will be no 
> indication that the multilib list came from GCC. Also, if things like the 
> `ExecuteAndWait` (in `getRISCVMultilibFromGCC`) fail shouldn't we print some 
> kind of diagnostic, at least in verbose mode? Otherwise when problems occur 
> it might be tricky to figure out what's going on.

Running an external program (`$triple-gcc`) to get library/include paths? This 
is a very unusual behavior to me and can have security issues. I'd suggest you 
raise a topic on cfe-dev getting consensus before this can be committed.

In the test, clang will invoke `python3 
Inputs/multilib_riscv64_elf_sdk/bin/riscv64-unknown-elf-gcc --print-multi-lib` 
and parse its output - this is unusual, too. I am concerned whether we can do 
this.

> So I think it would be great to just leverage that from GCC, the simplest way 
> is asking the multi-lib configuration from GCC.

The description should include an overview about what the patch does, not 
simply deferring to another place about what it will do. By following the link 
- the description does not clearly state what the particular GCC commit does as 
well.

Only by following the logic under a debugger it becomes clear to me that the 
patch adds code to spawn an external process of gcc, parse its output, and uses 
that as include/library/march/etc.

As an alternative, have you considered 
https://clang.llvm.org/docs/UsersManual.html#configuration-files




Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:395
+  "%0 option unrecognized in multi-lib configuration when parsing config "
+  "from GCC, falling back to built-in multi-lib configuration.">,
+  InGroup;

Delete trailing period.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:742
+  StringRef CodeModel;
+  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ))
+CodeModel = A->getValue();

getLastArgValue



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1602
+
+  // Find GCC from -gcc-toolchain if given.
+  if (const Arg *A =

`--gcc-toolchain` if specified

`-gcc-toolchain` is not a valid option.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1608
+// Try to find GCC from GCC_INSTALL_PREFIX if define.
+llvm::StringRef GCCInstallPrefix = GCC_INSTALL_PREFIX;
+std::string GCCPath;

This should reuse the `Generic_GCC::GCCInstallationDetector::init` logic.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1661
+
+  SmallVector Lines;
+  File.get()->getBuffer().split(Lines, "\n");

This means an inlin element number of 128. This is excessive - consumes lots of 
stack space. The value should typically be <= 4.



Comment at: clang/test/Driver/riscv-toolchain-gcc-multilib.c:6
+
+// RUN: %clang %s \
+// RUN:   -target riscv32-unknown-elf \

This can pack more options on one line.

Having more lines makes the file longer and actually harms readability.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97916

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

[clang] 28d58d8 - [Driver] Stop searching other prefixes once a GCC installation is found in one prefix

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-03-19T20:35:59-07:00
New Revision: 28d58d8fe2094af6902dee7b4d68ec30a3e9d737

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

LOG: [Driver] Stop searching other prefixes once a GCC installation is found in 
one prefix

so that when --sysroot is specified, the detected GCC installation will not be
overridden by another from /usr which happens to have a larger version.

This behavior is particularly inconvenient when the system has a larger version
GCC while the user wants to try out an older sysroot.

Delete some tests from linux-ld.c which overlap with cross-linux.c

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
clang/test/Driver/linux-ld.c
clang/unittests/Driver/ToolChainTest.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 38971288e38f..eb32f4b920b5 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1955,7 +1955,8 @@ void Generic_GCC::GCCInstallationDetector::init(
 
   // Loop over the various components which exist and select the best GCC
   // installation available. GCC installs are ranked by version number.
-  Version = GCCVersion::Parse("0.0.0");
+  const GCCVersion VersionZero = GCCVersion::Parse("0.0.0");
+  Version = VersionZero;
   for (const std::string  : Prefixes) {
 auto  = D.getVFS();
 if (!VFS.exists(Prefix))
@@ -1988,6 +1989,10 @@ void Generic_GCC::GCCInstallationDetector::init(
 ScanLibDirForGCCTriple(TargetTriple, Args, LibDir, Candidate, true,
GCCDirExists, GCCCrossDirExists);
 }
+
+// Skip other prefixes once a GCC installation is found.
+if (Version > VersionZero)
+  break;
   }
 }
 

diff  --git a/clang/test/Driver/linux-ld.c b/clang/test/Driver/linux-ld.c
index eba09d2970cc..1aa955737438 100644
--- a/clang/test/Driver/linux-ld.c
+++ b/clang/test/Driver/linux-ld.c
@@ -507,28 +507,6 @@
 // CHECK-64-TO-32-SYSROOT: "-L[[SYSROOT]]/lib"
 // CHECK-64-TO-32-SYSROOT: "-L[[SYSROOT]]/usr/lib"
 //
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: --target=i386-unknown-linux -rtlib=platform -m32 \
-// RUN: -ccc-install-dir %S/Inputs/fake_install_tree/bin \
-// RUN: --gcc-toolchain="" \
-// RUN: --sysroot=%S/Inputs/basic_linux_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-INSTALL-DIR-32 %s
-// CHECK-INSTALL-DIR-32: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-INSTALL-DIR-32: 
"{{.*}}/Inputs/fake_install_tree/bin/../lib/gcc/i386-unknown-linux/4.7.0{{/|}}crtbegin.o"
-// CHECK-INSTALL-DIR-32: 
"-L{{.*}}/Inputs/fake_install_tree/bin/../lib/gcc/i386-unknown-linux/4.7.0"
-//
-// Check that with 64-bit builds, we don't actually use the install directory
-// as its version of GCC is lower than our sysrooted version.
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: --target=x86_64-unknown-linux -rtlib=platform -m64 \
-// RUN: -ccc-install-dir %S/Inputs/fake_install_tree/bin \
-// RUN: --gcc-toolchain="" \
-// RUN: --sysroot=%S/Inputs/basic_linux_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-INSTALL-DIR-64 %s
-// CHECK-INSTALL-DIR-64: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-INSTALL-DIR-64: 
"{{.*}}/usr/lib/gcc/x86_64-unknown-linux/4.6.0{{/|}}crtbegin.o"
-// CHECK-INSTALL-DIR-64: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-unknown-linux/4.6.0"
-//
 // Check that we support unusual patch version formats, including missing that
 // component.
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -538,45 +516,8 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-GCC-VERSION1 %s
 // CHECK-GCC-VERSION1: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-GCC-VERSION1: 
"{{.*}}/Inputs/gcc_version_parsing1/bin/../lib/gcc/i386-unknown-linux/4.7{{/|}}crtbegin.o"
-// CHECK-GCC-VERSION1: 
"-L{{.*}}/Inputs/gcc_version_parsing1/bin/../lib/gcc/i386-unknown-linux/4.7"
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: --target=i386-unknown-linux -rtlib=platform -m32 \
-// RUN: -ccc-install-dir %S/Inputs/gcc_version_parsing2/bin \
-// RUN: --gcc-toolchain="" \
-// RUN: --sysroot=%S/Inputs/basic_linux_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-GCC-VERSION2 %s
-// CHECK-GCC-VERSION2: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-GCC-VERSION2: 
"{{.*}}/Inputs/gcc_version_parsing2/bin/../lib/gcc/i386-unknown-linux/4.7.x{{/|}}crtbegin.o"
-// CHECK-GCC-VERSION2: 
"-L{{.*}}/Inputs/gcc_version_parsing2/bin/../lib/gcc/i386-unknown-linux/4.7.x"
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: 

[PATCH] D99005: [clang] WIP: Implement P2266

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov created this revision.
Herald added subscribers: jansvoboda11, dexonsmith, lxfind, dang.
mizvekov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

WIP, not yet ready for review.

Small sample of things wrong with this patch:

- Have to remove some copypasta.
- Fix from D98971  directly imported here
- Some further simplifications needed.
- Test suite not updated to test new mode.
- In fact, the test suite was not run at all...

Just some manual testing:

- return: moderate
- throw: light
- co_return: not tested yet

Signed-off-by: Matheus Izvekov 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99005

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp

Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8839,6 +8839,10 @@
   if (E->isTypeDependent())
 return S.Context.DependentTy;
 
+  Expr *IDExpr = E;
+  if (auto ImplCastExpr = dyn_cast(E)) {
+IDExpr = ImplCastExpr->getSubExpr();
+  }
   // C++11 [dcl.type.simple]p4:
   //   The type denoted by decltype(e) is defined as follows:
 
@@ -8849,7 +8853,7 @@
   // Note that this does not pick up the implicit 'const' for a template
   // parameter object. This rule makes no difference before C++20 so we apply
   // it unconditionally.
-  if (const auto *SNTTPE = dyn_cast(E))
+  if (const auto *SNTTPE = dyn_cast(IDExpr))
 return SNTTPE->getParameterType(S.Context);
 
   // - if e is an unparenthesized id-expression or an unparenthesized class
@@ -8858,21 +8862,22 @@
   //   functions, the program is ill-formed;
   //
   // We apply the same rules for Objective-C ivar and property references.
-  if (const DeclRefExpr *DRE = dyn_cast(E)) {
+  if (const DeclRefExpr *DRE = dyn_cast(IDExpr)) {
 const ValueDecl *VD = DRE->getDecl();
 if (auto *TPO = dyn_cast(VD))
   return TPO->getType().getUnqualifiedType();
 return VD->getType();
-  } else if (const MemberExpr *ME = dyn_cast(E)) {
+  } else if (const MemberExpr *ME = dyn_cast(IDExpr)) {
 if (const ValueDecl *VD = ME->getMemberDecl())
   if (isa(VD) || isa(VD))
 return VD->getType();
-  } else if (const ObjCIvarRefExpr *IR = dyn_cast(E)) {
+  } else if (const ObjCIvarRefExpr *IR = dyn_cast(IDExpr)) {
 return IR->getDecl()->getType();
-  } else if (const ObjCPropertyRefExpr *PR = dyn_cast(E)) {
+  } else if (const ObjCPropertyRefExpr *PR =
+ dyn_cast(IDExpr)) {
 if (PR->isExplicitProperty())
   return PR->getExplicitProperty()->getType();
-  } else if (auto *PE = dyn_cast(E)) {
+  } else if (auto *PE = dyn_cast(IDExpr)) {
 return PE->getType();
   }
 
@@ -8885,8 +8890,8 @@
   //   entity.
   using namespace sema;
   if (S.getCurLambda()) {
-if (isa(E)) {
-  if (DeclRefExpr *DRE = dyn_cast(E->IgnoreParens())) {
+if (isa(IDExpr)) {
+  if (DeclRefExpr *DRE = dyn_cast(IDExpr->IgnoreParens())) {
 if (VarDecl *Var = dyn_cast(DRE->getDecl())) {
   QualType T = S.getCapturedDeclRefType(Var, DRE->getLocation());
   if (!T.isNull())
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3041,6 +3041,8 @@
 /// NRVO, or NULL if there is no such candidate.
 VarDecl *Sema::getCopyElisionCandidate(QualType ReturnType, Expr *E,
CopyElisionSemanticsKind CESK) {
+  if (auto ImplCastExpr = dyn_cast(E))
+E = ImplCastExpr->getSubExpr();
   // - in a return statement in a function [where] ...
   // ... the expression is the name of a non-volatile automatic object ...
   DeclRefExpr *DR = dyn_cast(E->IgnoreParens());
@@ -3086,15 +3088,14 @@
   if (VD->hasAttr())
 return false;
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  QualType VDNonRefType = VDType;
+  if (VDType->isReferenceType()) {
+if (!(CESK & CES_AllowRValueReferenceType) ||
+!VDType->isRValueReferenceType())
+  return false;
+VDNonRefType = VDType.getNonReferenceType();
+  }
+  if (!VDNonRefType->isObjectType() || VDNonRefType.isVolatileQualified())
 return false;
 
   if (CESK & CES_AllowDifferentTypes)
@@ -3102,8 +3103,8 @@
 
   // Variables with higher required alignment than their type's ABI
   // alignment cannot use NRVO.
-  if 

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

[Stephen Hines via Phabricator via cfe-commits]

srhines added a comment.

In D96403#2639180 , @srhines wrote:

> In D96403#2639137 , @smeenai wrote:
>
>> In D96403#2638932 , @rprichard 
>> wrote:
>>
>>> In D96403#2638883 , @smeenai wrote:
>>>
 With NDK r22, I only see libunwind.a for armv7. Will it be provided for 
 other architectures in future NDK versions?
>>>
>>> NDK r23 should have a libunwind.a for all architectures. This change isn't 
>>> in the r23 beta 2 that was just released, but it is in the current NDK 
>>> canary build at 
>>> https://ci.android.com/builds/branches/aosp-master-ndk/grid. (e.g. build 
>>> 7220482 
>>> ).
>>
>> Got it, thanks.
>>
>> Is there a public repository for viewing the libunwind source state that the 
>> NDK r23 libunwind is built from? I have the AOSP source tree checked out 
>> (following the instructions in 
>> https://source.android.com/setup/build/downloading) on the llvm-toolchain 
>> manifest branch. toolchain/llvm-project inside that checkout 
>> (https://android.googlesource.com/toolchain/llvm-project) has a branch for 
>> ndk-release-r23, but the merge base of that branch and upstream LLVM main is 
>> from April 29 2020, and I don't see many newer changes to libunwind after 
>> that, so I wasn't sure I was looking at the right place. I know you've done 
>> a bunch more libunwind work since then (e.g. D87750 
>> , D87881 , 
>> and D90898 ), so I was curious if those 
>> were gonna be part of r23's libunwind.
>
> So libunwind is now produced as a prebuilt with the rest of the toolchain 
> build, and not built from that copy of `toolchain/llvm_project` from the NDK 
> packaging/release branch. Thus if you want to rebuild/examine the sources, 
> you need to look at the toolchain that was included for NDK r23, which is 
> `clang-r399163b` (from 
> https://android.googlesource.com/platform/ndk/+/refs/tags/ndk-r23-beta2/ndk/toolchains.py#25).
>  Given that, you can get the manifest for the toolchain directly here 
> .
>  You can use that manifest to check out the exact sources that were used to 
> build everything (following instructions from here 
> ).
>
> ^ PROCESS is correct, but I picked the wrong version (not beta2). I'll update 
> and post a better reply in a few minutes.

It is actually using `clang-r416183` from toolchains.py 
.
 From there, you can go to the manifest for building those prebuilts 
.
 This line 

 shows you the SHA for the `toolchain/llvm-project` used to build the 
prebuilts. Finally, you can look here 

 to see what was merged. Note that there's a typo for the revision number in 
the commit message, but the upstream LLVM SHA 

 mentioned there is correct. Here 

 is where this toolchain was built in case that is useful/interesting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96403

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

hoy wrote:
> dblaikie wrote:
> > hoy wrote:
> > > dblaikie wrote:
> > > > hoy wrote:
> > > > > dblaikie wrote:
> > > > > > hoy wrote:
> > > > > > > dblaikie wrote:
> > > > > > > > Does this need to be down here? Or would the code be a well 
> > > > > > > > exercised if it was up next to the go declaration above?
> > > > > > > Yes, it needs to be here. Otherwise it will just like the 
> > > > > > > function `bar` above that doesn't get a uniquefied name. I think 
> > > > > > > moving the definition up to right after the declaration hides the 
> > > > > > > declaration.
> > > > > > Not sure I follow - do you mean that if the go declaration and go 
> > > > > > definition were next to each other, this test would (mechanically 
> > > > > > speaking) not validate what the patch? Or that it would be less 
> > > > > > legible, but still mechanically correct?
> > > > > > 
> > > > > > I think it would be (assuming it's still mechanically correct) more 
> > > > > > legible to put the declaration next to the definition - the comment 
> > > > > > describes why the declaration is significant/why the definition is 
> > > > > > weird, and seeing all that together would be clearer to me than 
> > > > > > spreading it out/having to look further away to see what's going on.
> > > > > When the `go` declaration and `go` definition were next to each 
> > > > > other, the go function won't get a uniqufied name at all. The 
> > > > > declaration will be overwritten by the definition. Only when the 
> > > > > declaration is seen by others, such the callsite in `baz`, the 
> > > > > declaration makes a difference by having the callsite use a uniqufied 
> > > > > name.
> > > > > 
> > > > > 
> > > > Ah! Interesting, good to know. 
> > > > 
> > > > Is that worth supporting, I wonder? I guess it falls out for 
> > > > free/without significant additional complexity. I worry about the 
> > > > subtlety of the additional declaration changing the behavior here... 
> > > > might be a bit surprising/subtle. But maybe no nice way to avoid it 
> > > > either.
> > > It would be ideal if user never writes code like that. Unfortunately it 
> > > exists with legacy code (such as mysql). I think it's worth supporting it 
> > > from AutoFDO point of view to avoid a silent mismatch between debug 
> > > linkage name and real linkage name.
> > Oh, I agree that we shouldn't mismatch debug info and the actual symbol 
> > name - what I meant was whether code like this should get mangled or not 
> > when using unique-internal-linkage names.
> > 
> > I'm now more curious about this:
> > 
> > > When the `go` declaration and `go` definition were next to each other, 
> > > the go function won't get a uniqufied name at all.
> > 
> > This doesn't seem to happen with the `__attribute__((overloadable))` 
> > attribute, for instance - so any idea what's different about uniquification 
> > that's working differently than overloadable?
> > 
> > ```
> > $ cat test.c
> > __attribute__((overloadable)) static int go(a) int a; {
> >   return 3 + a;
> > }
> > void baz() {
> >   go(2);
> > }
> > $ clang-tot test.c -emit-llvm -S -o - | grep go
> >   %call = call i32 @_ZL2goi(i32 2)
> > define internal i32 @_ZL2goi(i32 %a) #0 {
> > ```
> Good question. I'm not sure what's exactly going on but it looks like with 
> the overloadable attribute, the old-style definition is treated as having 
> prototype. But if you do this:
> 
> ```
> __attribute__((overloadable)) 
> void baz() {}
> ```
> then there's the error:
> 
> ```
> error: 'overloadable' function 'baz' must have a prototype
> void baz() {
> ```
> 
> `void baz() {` does not come with a prototype. That's for sure.  Sounds like 
> `int go(a) int a {;` can have a prototype when it is loadable. I'm wondering 
> why it's not always treated as having prototype, since the parameter type is 
> there.
> 
> 
> 
Yeah, that seems like that divergence be worth understanding (& if possible 
fixing/avoiding/merging). Ensuring these features don't have subtle divergence 
I think will be valuable to having a model that's easy to 
explain/understand/modify/etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

[Stephen Hines via Phabricator via cfe-commits]

srhines added a comment.

In D96403#2639137 , @smeenai wrote:

> In D96403#2638932 , @rprichard wrote:
>
>> In D96403#2638883 , @smeenai wrote:
>>
>>> With NDK r22, I only see libunwind.a for armv7. Will it be provided for 
>>> other architectures in future NDK versions?
>>
>> NDK r23 should have a libunwind.a for all architectures. This change isn't 
>> in the r23 beta 2 that was just released, but it is in the current NDK 
>> canary build at https://ci.android.com/builds/branches/aosp-master-ndk/grid. 
>> (e.g. build 7220482 
>> ).
>
> Got it, thanks.
>
> Is there a public repository for viewing the libunwind source state that the 
> NDK r23 libunwind is built from? I have the AOSP source tree checked out 
> (following the instructions in 
> https://source.android.com/setup/build/downloading) on the llvm-toolchain 
> manifest branch. toolchain/llvm-project inside that checkout 
> (https://android.googlesource.com/toolchain/llvm-project) has a branch for 
> ndk-release-r23, but the merge base of that branch and upstream LLVM main is 
> from April 29 2020, and I don't see many newer changes to libunwind after 
> that, so I wasn't sure I was looking at the right place. I know you've done a 
> bunch more libunwind work since then (e.g. D87750 
> , D87881 , 
> and D90898 ), so I was curious if those were 
> gonna be part of r23's libunwind.

So libunwind is now produced as a prebuilt with the rest of the toolchain 
build, and not built from that copy of `toolchain/llvm_project` from the NDK 
packaging/release branch. Thus if you want to rebuild/examine the sources, you 
need to look at the toolchain that was included for NDK r23, which is 
`clang-r399163b` (from 
https://android.googlesource.com/platform/ndk/+/refs/tags/ndk-r23-beta2/ndk/toolchains.py#25).
 Given that, you can get the manifest for the toolchain directly here 
.
 You can use that manifest to check out the exact sources that were used to 
build everything (following instructions from here 
).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96403

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

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In D96403#2638932 , @rprichard wrote:

> In D96403#2638883 , @smeenai wrote:
>
>> With NDK r22, I only see libunwind.a for armv7. Will it be provided for 
>> other architectures in future NDK versions?
>
> NDK r23 should have a libunwind.a for all architectures. This change isn't in 
> the r23 beta 2 that was just released, but it is in the current NDK canary 
> build at https://ci.android.com/builds/branches/aosp-master-ndk/grid. (e.g. 
> build 7220482 
> ).

Got it, thanks.

Is there a public repository for viewing the libunwind source state that the 
NDK r23 libunwind is built from? I have the AOSP source tree checked out 
(following the instructions in 
https://source.android.com/setup/build/downloading) on the llvm-toolchain 
manifest branch. toolchain/llvm-project inside that checkout 
(https://android.googlesource.com/toolchain/llvm-project) has a branch for 
ndk-release-r23, but the merge base of that branch and upstream LLVM main is 
from April 29 2020, and I don't see many newer changes to libunwind after that, 
so I wasn't sure I was looking at the right place. I know you've done a bunch 
more libunwind work since then (e.g. D87750 , 
D87881 , and D90898 
), so I was curious if those were gonna be 
part of r23's libunwind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96403

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

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

Quuxplusone wrote:
> mizvekov wrote:
> > aaronpuchert wrote:
> > > mizvekov wrote:
> > > > Quuxplusone wrote:
> > > > > mizvekov wrote:
> > > > > > mizvekov wrote:
> > > > > > > mizvekov wrote:
> > > > > > > > A drive by fix here would be that we already have a VDType in 
> > > > > > > > this context, might as well use it even though original for 
> > > > > > > > some reason missed it in this part.
> > > > > > > This whole block is also logically equivalent to the much simpler:
> > > > > > > ```
> > > > > > > if (VDType.isReferenceType()) {
> > > > > > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > > > > > !VDType.isRValueReferenceType())
> > > > > > >   return false;
> > > > > > > VDType = VDType.getNonReferenceType()
> > > > > > > }
> > > > > > > if (!VDType.isObjectType() || VDType.isVolatileQualified()) 
> > > > > > >   return false;
> > > > > > > ```
> > > > > > > But you do have to adjust the comments there and adjust the rest 
> > > > > > > to use VDType consistently :)
> > > > > > > Also, I think it might be possible to even remove the 
> > > > > > > `!VDType.isObjectType() || ` part from my suggestion above, 
> > > > > > > because it might be the only option left if it is not a reference 
> > > > > > > anyway.
> > > > > > ```
> > > > > >   bool isObjectType() const {
> > > > > > // C++ [basic.types]p8:
> > > > > > //   An object type is a (possibly cv-qualified) type that is 
> > > > > > not a
> > > > > > //   function type, not a reference type, and not a void type.
> > > > > > return !isReferenceType() && !isFunctionType() && !isVoidType();
> > > > > >   }
> > > > > > ```
> > > > > > So yeah I think you can just make my suggestion be:
> > > > > > ```
> > > > > > if (VDType.isReferenceType()) {
> > > > > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > > > > !VDType.isRValueReferenceType())
> > > > > >   return false;
> > > > > > VDType = VDType.getNonReferenceType()
> > > > > > }
> > > > > > if (VDType.isVolatileQualified()) 
> > > > > >   return false;
> > > > > > ```
> > > > > > 
> > > > > > 
> > > > > Yeah but I //reaally// don't want to
> > > > > (1) change the meaning of `VDType` in the middle of this function 
> > > > > (mantra: "one name = one meaning")
> > > > > (2) get "clever". I don't want to have to think about "Are there any 
> > > > > types that are neither object types nor reference types?" (What about 
> > > > > function types? What about block types? What about, I dunno, 
> > > > > bit-fields?) I want the code to be //obviously correct//, and also to 
> > > > > casewise match exactly what the standard says. It says object or 
> > > > > rvalue reference type — let's write code that expresses that wording 
> > > > > //exactly//.
> > > > How about:
> > > > ```
> > > > QualType VDObjType = VDType;
> > > > if (!VDType.isObjectType()) {
> > > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > > !VDType.isRValueReferenceType())
> > > >   return false;
> > > > VDObjType = VDType.getNonReferenceType();
> > > > }
> > > > if (VDObjType .isVolatileQualified()) 
> > > >   return false;
> > > > ```
> > > > And then `s/VDType/VDObjType/` from here on.
> > > > I think this expresses the meaning of the standard clearly.
> > > That seems like a sensible simplification, the proposed code is indeed a 
> > > bit repetitive. I'd go with the original suggestion plus the new variable:
> > > 
> > > ```
> > > QualType VDNonRefType = VDType;
> > > if (VDType.isReferenceType()) {
> > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > !VDType.isRValueReferenceType())
> > >   return false;
> > > VDNonRefType = VDType.getNonReferenceType()
> > > }
> > > if (!VDNonRefType.isObjectType() || VDNonRefType.isVolatileQualified()) 
> > >   return false;
> > > ```
> > > 
> > > or whatever name you find appropriate. Actually it's the type of the 
> > > `DeclRefExpr`, isn't it? So maybe `DREType`?
> > > 
> > > The initialization might be a bit misleading, an alternative would be to 
> > > not initialize and have an assignment `VDNonRefType = VDType` in the else 
> > > branch instead.
> > @aaronpuchert Yeah that combination is good to me also, and I liked the 
> > name you suggested better. So +1 :)
> > Actually it's the type of the `DeclRefExpr`, isn't it? So maybe `DREType`?
> 
> The fact that we don't know what it is gives me pause, re introducing it. ;) 
> If I were going to introduce a local synonym for 

[PATCH] D98429: [clang-format] Add new option to clang-format: SpaceBeforeForLoopSemiColon

[Nathan Hjelm via Phabricator via cfe-commits]

hjelmn added a comment.

This is something I have been doing for over 20 years now. Not sure when I 
initially picked it up but I find a space before the ;'s in a C for loop 
improves readability. It more clearly differentiates the different parts. I 
beleive the space before the colon in C++ range-based loops is based on the 
same readability improvement.

Anyway, I intend to use clang-format in a couple of C projects and want to make 
sure it doesn't try to remove the intentional formatting of the code. I have at 
least one more CL I need to open up for a bug I found in the formatting. Will 
try to get that one open as soon as I get this one finished.




Comment at: clang/include/clang/Format/Format.h:2841
+  ///true:  false:
+  ///for (i = 0 ; i < 1 ; ++i) {}   vs. for (i = 0; i < 1; ++i) {}
+  /// \endcode

HazardyKnusperkeks wrote:
> Please also show the range based for here. Otherwise it may be surprising, it 
> would be for me.
Will do. Plan to get back to this this weekend.



Comment at: clang/lib/Format/TokenAnnotator.cpp:4004
 return false;
+  if (Right.is(TT_ForLoopSemiColon))
+return false;

curdeius wrote:
> Is this change really necessary? Is there a test for it?
> I guess that a semicolon should return false because otherwise it could be 
> put after the line break, but you certainly need a test for that. 
Yes. The way I implemented it this I think it was necessary. I will have to 
double-check. I plan to re-do this with an enum so will try to avoid extraneous 
changes.



Comment at: clang/unittests/Format/FormatTest.cpp:12712
+  verifyFormat("for (i = 0 ; i < 10 ; ++i) {\n}", Space);
+  verifyFormat("for (int i = 0 ; auto a : b) {\n}", Space);
+}

curdeius wrote:
> HazardyKnusperkeks wrote:
> > hjelmn wrote:
> > > HazardyKnusperkeks wrote:
> > > > Okay that was unexpected for me, I thought the space would only apply 
> > > > to the old `for`s.
> > > > 
> > > > In that case please add `while` and maybe `if` with initializer. What 
> > > > should be discussed is if `for` and the other control statements with 
> > > > initializer should behave differently with regard to their initializer 
> > > > semicolon.
> > > hmm, I think then I could rename this one to: 
> > > SpaceBeforeCForLoopSemiColon which would then only add spaces in 
> > > for(init;cond;increment)
> > > then maybe SpaceAfterControlStatementInitializer or 
> > > SpaceBeforeControlStatementSemiColon that controls the behavior for 
> > > control statements with initializers.
> > > 
> > > How does that sound?
> > Apart from the names good. For the names I don't have anything really 
> > better right now.
> > 
> > But this is currently just my opinion, we should ask @MyDeveloperDay and 
> > @curdeius.
> I would prefer to avoid multiplying the different options and regroup all of 
> the control statements under a single one.
> `SpaceBeforeControlStatementSemicolon` sounds acceptable with one nit, I'd 
> use "Semicolon" (as a single word, with lowercase 'c') to match the usage in 
> LLVM (e.g. in clang-tidy).
> 
> WDYT about a single option for all statements?
> Another way is to add a separate option for each statement type. Or, use a 
> (bitmask like) enum with a single option. The latter can be done in a 
> backward compatible way in the future BTW.
Makes sense to me. Will do that and will close this and other comment once that 
is complete.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98429

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

dblaikie wrote:
> hoy wrote:
> > dblaikie wrote:
> > > hoy wrote:
> > > > dblaikie wrote:
> > > > > hoy wrote:
> > > > > > dblaikie wrote:
> > > > > > > Does this need to be down here? Or would the code be a well 
> > > > > > > exercised if it was up next to the go declaration above?
> > > > > > Yes, it needs to be here. Otherwise it will just like the function 
> > > > > > `bar` above that doesn't get a uniquefied name. I think moving the 
> > > > > > definition up to right after the declaration hides the declaration.
> > > > > Not sure I follow - do you mean that if the go declaration and go 
> > > > > definition were next to each other, this test would (mechanically 
> > > > > speaking) not validate what the patch? Or that it would be less 
> > > > > legible, but still mechanically correct?
> > > > > 
> > > > > I think it would be (assuming it's still mechanically correct) more 
> > > > > legible to put the declaration next to the definition - the comment 
> > > > > describes why the declaration is significant/why the definition is 
> > > > > weird, and seeing all that together would be clearer to me than 
> > > > > spreading it out/having to look further away to see what's going on.
> > > > When the `go` declaration and `go` definition were next to each other, 
> > > > the go function won't get a uniqufied name at all. The declaration will 
> > > > be overwritten by the definition. Only when the declaration is seen by 
> > > > others, such the callsite in `baz`, the declaration makes a difference 
> > > > by having the callsite use a uniqufied name.
> > > > 
> > > > 
> > > Ah! Interesting, good to know. 
> > > 
> > > Is that worth supporting, I wonder? I guess it falls out for free/without 
> > > significant additional complexity. I worry about the subtlety of the 
> > > additional declaration changing the behavior here... might be a bit 
> > > surprising/subtle. But maybe no nice way to avoid it either.
> > It would be ideal if user never writes code like that. Unfortunately it 
> > exists with legacy code (such as mysql). I think it's worth supporting it 
> > from AutoFDO point of view to avoid a silent mismatch between debug linkage 
> > name and real linkage name.
> Oh, I agree that we shouldn't mismatch debug info and the actual symbol name 
> - what I meant was whether code like this should get mangled or not when 
> using unique-internal-linkage names.
> 
> I'm now more curious about this:
> 
> > When the `go` declaration and `go` definition were next to each other, the 
> > go function won't get a uniqufied name at all.
> 
> This doesn't seem to happen with the `__attribute__((overloadable))` 
> attribute, for instance - so any idea what's different about uniquification 
> that's working differently than overloadable?
> 
> ```
> $ cat test.c
> __attribute__((overloadable)) static int go(a) int a; {
>   return 3 + a;
> }
> void baz() {
>   go(2);
> }
> $ clang-tot test.c -emit-llvm -S -o - | grep go
>   %call = call i32 @_ZL2goi(i32 2)
> define internal i32 @_ZL2goi(i32 %a) #0 {
> ```
Good question. I'm not sure what's exactly going on but it looks like with the 
overloadable attribute, the old-style definition is treated as having 
prototype. But if you do this:

```
__attribute__((overloadable)) 
void baz() {}
```
then there's the error:

```
error: 'overloadable' function 'baz' must have a prototype
void baz() {
```

`void baz() {` does not come with a prototype. That's for sure.  Sounds like 
`int go(a) int a {;` can have a prototype when it is loadable. I'm wondering 
why it's not always treated as having prototype, since the parameter type is 
there.





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Arthur O'Dwyer via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

mizvekov wrote:
> aaronpuchert wrote:
> > mizvekov wrote:
> > > Quuxplusone wrote:
> > > > mizvekov wrote:
> > > > > mizvekov wrote:
> > > > > > mizvekov wrote:
> > > > > > > A drive by fix here would be that we already have a VDType in 
> > > > > > > this context, might as well use it even though original for some 
> > > > > > > reason missed it in this part.
> > > > > > This whole block is also logically equivalent to the much simpler:
> > > > > > ```
> > > > > > if (VDType.isReferenceType()) {
> > > > > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > > > > !VDType.isRValueReferenceType())
> > > > > >   return false;
> > > > > > VDType = VDType.getNonReferenceType()
> > > > > > }
> > > > > > if (!VDType.isObjectType() || VDType.isVolatileQualified()) 
> > > > > >   return false;
> > > > > > ```
> > > > > > But you do have to adjust the comments there and adjust the rest to 
> > > > > > use VDType consistently :)
> > > > > > Also, I think it might be possible to even remove the 
> > > > > > `!VDType.isObjectType() || ` part from my suggestion above, because 
> > > > > > it might be the only option left if it is not a reference anyway.
> > > > > ```
> > > > >   bool isObjectType() const {
> > > > > // C++ [basic.types]p8:
> > > > > //   An object type is a (possibly cv-qualified) type that is not 
> > > > > a
> > > > > //   function type, not a reference type, and not a void type.
> > > > > return !isReferenceType() && !isFunctionType() && !isVoidType();
> > > > >   }
> > > > > ```
> > > > > So yeah I think you can just make my suggestion be:
> > > > > ```
> > > > > if (VDType.isReferenceType()) {
> > > > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > > > !VDType.isRValueReferenceType())
> > > > >   return false;
> > > > > VDType = VDType.getNonReferenceType()
> > > > > }
> > > > > if (VDType.isVolatileQualified()) 
> > > > >   return false;
> > > > > ```
> > > > > 
> > > > > 
> > > > Yeah but I //reaally// don't want to
> > > > (1) change the meaning of `VDType` in the middle of this function 
> > > > (mantra: "one name = one meaning")
> > > > (2) get "clever". I don't want to have to think about "Are there any 
> > > > types that are neither object types nor reference types?" (What about 
> > > > function types? What about block types? What about, I dunno, 
> > > > bit-fields?) I want the code to be //obviously correct//, and also to 
> > > > casewise match exactly what the standard says. It says object or rvalue 
> > > > reference type — let's write code that expresses that wording 
> > > > //exactly//.
> > > How about:
> > > ```
> > > QualType VDObjType = VDType;
> > > if (!VDType.isObjectType()) {
> > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > !VDType.isRValueReferenceType())
> > >   return false;
> > > VDObjType = VDType.getNonReferenceType();
> > > }
> > > if (VDObjType .isVolatileQualified()) 
> > >   return false;
> > > ```
> > > And then `s/VDType/VDObjType/` from here on.
> > > I think this expresses the meaning of the standard clearly.
> > That seems like a sensible simplification, the proposed code is indeed a 
> > bit repetitive. I'd go with the original suggestion plus the new variable:
> > 
> > ```
> > QualType VDNonRefType = VDType;
> > if (VDType.isReferenceType()) {
> > if (!(CESK & CES_AllowRValueReferenceType) || 
> > !VDType.isRValueReferenceType())
> >   return false;
> > VDNonRefType = VDType.getNonReferenceType()
> > }
> > if (!VDNonRefType.isObjectType() || VDNonRefType.isVolatileQualified()) 
> >   return false;
> > ```
> > 
> > or whatever name you find appropriate. Actually it's the type of the 
> > `DeclRefExpr`, isn't it? So maybe `DREType`?
> > 
> > The initialization might be a bit misleading, an alternative would be to 
> > not initialize and have an assignment `VDNonRefType = VDType` in the else 
> > branch instead.
> @aaronpuchert Yeah that combination is good to me also, and I liked the name 
> you suggested better. So +1 :)
> Actually it's the type of the `DeclRefExpr`, isn't it? So maybe `DREType`?

The fact that we don't know what it is gives me pause, re introducing it. ;) If 
I were going to introduce a local synonym for `VDType.getNonReferenceType()` on 
lines 3105-3108, I guess `VDReferencedType` would be the best name for it; but 
I don't think there's any reason to introduce another name here.

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

aaronpuchert wrote:
> mizvekov wrote:
> > Quuxplusone wrote:
> > > mizvekov wrote:
> > > > mizvekov wrote:
> > > > > mizvekov wrote:
> > > > > > A drive by fix here would be that we already have a VDType in this 
> > > > > > context, might as well use it even though original for some reason 
> > > > > > missed it in this part.
> > > > > This whole block is also logically equivalent to the much simpler:
> > > > > ```
> > > > > if (VDType.isReferenceType()) {
> > > > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > > > !VDType.isRValueReferenceType())
> > > > >   return false;
> > > > > VDType = VDType.getNonReferenceType()
> > > > > }
> > > > > if (!VDType.isObjectType() || VDType.isVolatileQualified()) 
> > > > >   return false;
> > > > > ```
> > > > > But you do have to adjust the comments there and adjust the rest to 
> > > > > use VDType consistently :)
> > > > > Also, I think it might be possible to even remove the 
> > > > > `!VDType.isObjectType() || ` part from my suggestion above, because 
> > > > > it might be the only option left if it is not a reference anyway.
> > > > ```
> > > >   bool isObjectType() const {
> > > > // C++ [basic.types]p8:
> > > > //   An object type is a (possibly cv-qualified) type that is not a
> > > > //   function type, not a reference type, and not a void type.
> > > > return !isReferenceType() && !isFunctionType() && !isVoidType();
> > > >   }
> > > > ```
> > > > So yeah I think you can just make my suggestion be:
> > > > ```
> > > > if (VDType.isReferenceType()) {
> > > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > > !VDType.isRValueReferenceType())
> > > >   return false;
> > > > VDType = VDType.getNonReferenceType()
> > > > }
> > > > if (VDType.isVolatileQualified()) 
> > > >   return false;
> > > > ```
> > > > 
> > > > 
> > > Yeah but I //reaally// don't want to
> > > (1) change the meaning of `VDType` in the middle of this function 
> > > (mantra: "one name = one meaning")
> > > (2) get "clever". I don't want to have to think about "Are there any 
> > > types that are neither object types nor reference types?" (What about 
> > > function types? What about block types? What about, I dunno, bit-fields?) 
> > > I want the code to be //obviously correct//, and also to casewise match 
> > > exactly what the standard says. It says object or rvalue reference type — 
> > > let's write code that expresses that wording //exactly//.
> > How about:
> > ```
> > QualType VDObjType = VDType;
> > if (!VDType.isObjectType()) {
> > if (!(CESK & CES_AllowRValueReferenceType) || 
> > !VDType.isRValueReferenceType())
> >   return false;
> > VDObjType = VDType.getNonReferenceType();
> > }
> > if (VDObjType .isVolatileQualified()) 
> >   return false;
> > ```
> > And then `s/VDType/VDObjType/` from here on.
> > I think this expresses the meaning of the standard clearly.
> That seems like a sensible simplification, the proposed code is indeed a bit 
> repetitive. I'd go with the original suggestion plus the new variable:
> 
> ```
> QualType VDNonRefType = VDType;
> if (VDType.isReferenceType()) {
> if (!(CESK & CES_AllowRValueReferenceType) || 
> !VDType.isRValueReferenceType())
>   return false;
> VDNonRefType = VDType.getNonReferenceType()
> }
> if (!VDNonRefType.isObjectType() || VDNonRefType.isVolatileQualified()) 
>   return false;
> ```
> 
> or whatever name you find appropriate. Actually it's the type of the 
> `DeclRefExpr`, isn't it? So maybe `DREType`?
> 
> The initialization might be a bit misleading, an alternative would be to not 
> initialize and have an assignment `VDNonRefType = VDType` in the else branch 
> instead.
@aaronpuchert Yeah that combination is good to me also, and I liked the name 
you suggested better. So +1 :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

hoy wrote:
> dblaikie wrote:
> > hoy wrote:
> > > dblaikie wrote:
> > > > hoy wrote:
> > > > > dblaikie wrote:
> > > > > > Does this need to be down here? Or would the code be a well 
> > > > > > exercised if it was up next to the go declaration above?
> > > > > Yes, it needs to be here. Otherwise it will just like the function 
> > > > > `bar` above that doesn't get a uniquefied name. I think moving the 
> > > > > definition up to right after the declaration hides the declaration.
> > > > Not sure I follow - do you mean that if the go declaration and go 
> > > > definition were next to each other, this test would (mechanically 
> > > > speaking) not validate what the patch? Or that it would be less 
> > > > legible, but still mechanically correct?
> > > > 
> > > > I think it would be (assuming it's still mechanically correct) more 
> > > > legible to put the declaration next to the definition - the comment 
> > > > describes why the declaration is significant/why the definition is 
> > > > weird, and seeing all that together would be clearer to me than 
> > > > spreading it out/having to look further away to see what's going on.
> > > When the `go` declaration and `go` definition were next to each other, 
> > > the go function won't get a uniqufied name at all. The declaration will 
> > > be overwritten by the definition. Only when the declaration is seen by 
> > > others, such the callsite in `baz`, the declaration makes a difference by 
> > > having the callsite use a uniqufied name.
> > > 
> > > 
> > Ah! Interesting, good to know. 
> > 
> > Is that worth supporting, I wonder? I guess it falls out for free/without 
> > significant additional complexity. I worry about the subtlety of the 
> > additional declaration changing the behavior here... might be a bit 
> > surprising/subtle. But maybe no nice way to avoid it either.
> It would be ideal if user never writes code like that. Unfortunately it 
> exists with legacy code (such as mysql). I think it's worth supporting it 
> from AutoFDO point of view to avoid a silent mismatch between debug linkage 
> name and real linkage name.
Oh, I agree that we shouldn't mismatch debug info and the actual symbol name - 
what I meant was whether code like this should get mangled or not when using 
unique-internal-linkage names.

I'm now more curious about this:

> When the `go` declaration and `go` definition were next to each other, the go 
> function won't get a uniqufied name at all.

This doesn't seem to happen with the `__attribute__((overloadable))` attribute, 
for instance - so any idea what's different about uniquification that's working 
differently than overloadable?

```
$ cat test.c
__attribute__((overloadable)) static int go(a) int a; {
  return 3 + a;
}
void baz() {
  go(2);
}
$ clang-tot test.c -emit-llvm -S -o - | grep go
  %call = call i32 @_ZL2goi(i32 2)
define internal i32 @_ZL2goi(i32 %a) #0 {
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

dblaikie wrote:
> hoy wrote:
> > dblaikie wrote:
> > > hoy wrote:
> > > > dblaikie wrote:
> > > > > Does this need to be down here? Or would the code be a well exercised 
> > > > > if it was up next to the go declaration above?
> > > > Yes, it needs to be here. Otherwise it will just like the function 
> > > > `bar` above that doesn't get a uniquefied name. I think moving the 
> > > > definition up to right after the declaration hides the declaration.
> > > Not sure I follow - do you mean that if the go declaration and go 
> > > definition were next to each other, this test would (mechanically 
> > > speaking) not validate what the patch? Or that it would be less legible, 
> > > but still mechanically correct?
> > > 
> > > I think it would be (assuming it's still mechanically correct) more 
> > > legible to put the declaration next to the definition - the comment 
> > > describes why the declaration is significant/why the definition is weird, 
> > > and seeing all that together would be clearer to me than spreading it 
> > > out/having to look further away to see what's going on.
> > When the `go` declaration and `go` definition were next to each other, the 
> > go function won't get a uniqufied name at all. The declaration will be 
> > overwritten by the definition. Only when the declaration is seen by others, 
> > such the callsite in `baz`, the declaration makes a difference by having 
> > the callsite use a uniqufied name.
> > 
> > 
> Ah! Interesting, good to know. 
> 
> Is that worth supporting, I wonder? I guess it falls out for free/without 
> significant additional complexity. I worry about the subtlety of the 
> additional declaration changing the behavior here... might be a bit 
> surprising/subtle. But maybe no nice way to avoid it either.
It would be ideal if user never writes code like that. Unfortunately it exists 
with legacy code (such as mysql). I think it's worth supporting it from AutoFDO 
point of view to avoid a silent mismatch between debug linkage name and real 
linkage name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[clang] f9cac39 - [Driver] Delete compatibility aliases -mpie-copy-relocations and -mno-pie-copy-relocations

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-03-19T17:47:30-07:00
New Revision: f9cac39930c8ae13892f8daa8662e4ec65439f22

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

LOG: [Driver] Delete compatibility aliases -mpie-copy-relocations and 
-mno-pie-copy-relocations

They should be unused everywhere.

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Driver/Options.td
clang/test/Driver/fdirect-access-external-data.c

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 962d717483e0..d895587c458a 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -2797,10 +2797,6 @@ Use packed stack layout (SystemZ only).
 
 Specify maximum number of prefixes to use for padding
 
-.. option:: -mpie-copy-relocations, -mno-pie-copy-relocations
-
-Use copy relocations support for PIE builds
-
 .. option:: -mprefer-vector-width=
 
 Specifies preferred vector width for auto-vectorization. Defaults to 'none' 
which allows target specific decisions.

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 85a0e02e6357..6e22bd01bea3 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3323,10 +3323,6 @@ def mstack_protector_guard_offset_EQ : Joined<["-"], 
"mstack-protector-guard-off
 def mstack_protector_guard_reg_EQ : Joined<["-"], 
"mstack-protector-guard-reg=">, Group, Flags<[CC1Option]>,
   HelpText<"Use the given reg for addressing the stack-protector guard">,
   MarshallingInfoString, [{"none"}]>;
-def mpie_copy_relocations : Flag<["-"], "mpie-copy-relocations">,
-  Alias, Group;
-def mno_pie_copy_relocations : Flag<["-"], "mno-pie-copy-relocations">,
-  Alias, Group;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at 
function entry (x86/SystemZ only)">,
   Flags<[CC1Option]>, Group,
   MarshallingInfoFlag>;

diff  --git a/clang/test/Driver/fdirect-access-external-data.c 
b/clang/test/Driver/fdirect-access-external-data.c
index c3fc93064179..f132b1b088af 100644
--- a/clang/test/Driver/fdirect-access-external-data.c
+++ b/clang/test/Driver/fdirect-access-external-data.c
@@ -9,10 +9,6 @@
 // RUN: %clang -### -c -target aarch64 %s -fpic 2>&1 | FileCheck %s 
--check-prefix=DEFAULT
 // RUN: %clang -### -c -target aarch64 %s -fpic -fdirect-access-external-data 
2>&1 | FileCheck %s --check-prefix=DIRECT
 
-/// -m[no-]pie-copy-relocations are aliases for compatibility.
-// RUN: %clang -### -c -target riscv64 %s -mno-pie-copy-relocations 2>&1 | 
FileCheck %s --check-prefix=INDIRECT
-// RUN: %clang -### -c -target riscv64 %s -fpic -mpie-copy-relocations 2>&1 | 
FileCheck %s --check-prefix=DIRECT
-
 // DEFAULT-NOT: direct-access-external-data"
 // DIRECT:  "-fdirect-access-external-data"
 // INDIRECT:"-fno-direct-access-external-data"



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

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3102
+// C++20 [class.copy.elision]p3:
+// ...either a non-volatile object or an rvalue reference to a 
non-volatile object type...
+if (!(CESK & CES_AllowRValueReferenceType))

This is probably over the line limit, maybe try to reflow this as suggested.



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

mizvekov wrote:
> Quuxplusone wrote:
> > mizvekov wrote:
> > > mizvekov wrote:
> > > > mizvekov wrote:
> > > > > A drive by fix here would be that we already have a VDType in this 
> > > > > context, might as well use it even though original for some reason 
> > > > > missed it in this part.
> > > > This whole block is also logically equivalent to the much simpler:
> > > > ```
> > > > if (VDType.isReferenceType()) {
> > > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > > !VDType.isRValueReferenceType())
> > > >   return false;
> > > > VDType = VDType.getNonReferenceType()
> > > > }
> > > > if (!VDType.isObjectType() || VDType.isVolatileQualified()) 
> > > >   return false;
> > > > ```
> > > > But you do have to adjust the comments there and adjust the rest to use 
> > > > VDType consistently :)
> > > > Also, I think it might be possible to even remove the 
> > > > `!VDType.isObjectType() || ` part from my suggestion above, because it 
> > > > might be the only option left if it is not a reference anyway.
> > > ```
> > >   bool isObjectType() const {
> > > // C++ [basic.types]p8:
> > > //   An object type is a (possibly cv-qualified) type that is not a
> > > //   function type, not a reference type, and not a void type.
> > > return !isReferenceType() && !isFunctionType() && !isVoidType();
> > >   }
> > > ```
> > > So yeah I think you can just make my suggestion be:
> > > ```
> > > if (VDType.isReferenceType()) {
> > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > !VDType.isRValueReferenceType())
> > >   return false;
> > > VDType = VDType.getNonReferenceType()
> > > }
> > > if (VDType.isVolatileQualified()) 
> > >   return false;
> > > ```
> > > 
> > > 
> > Yeah but I //reaally// don't want to
> > (1) change the meaning of `VDType` in the middle of this function (mantra: 
> > "one name = one meaning")
> > (2) get "clever". I don't want to have to think about "Are there any types 
> > that are neither object types nor reference types?" (What about function 
> > types? What about block types? What about, I dunno, bit-fields?) I want the 
> > code to be //obviously correct//, and also to casewise match exactly what 
> > the standard says. It says object or rvalue reference type — let's write 
> > code that expresses that wording //exactly//.
> How about:
> ```
> QualType VDObjType = VDType;
> if (!VDType.isObjectType()) {
> if (!(CESK & CES_AllowRValueReferenceType) || 
> !VDType.isRValueReferenceType())
>   return false;
> VDObjType = VDType.getNonReferenceType();
> }
> if (VDObjType .isVolatileQualified()) 
>   return false;
> ```
> And then `s/VDType/VDObjType/` from here on.
> I think this expresses the meaning of the standard clearly.
That seems like a sensible simplification, the proposed code is indeed a bit 
repetitive. I'd go with the original suggestion plus the new variable:

```
QualType VDNonRefType = VDType;
if (VDType.isReferenceType()) {
if (!(CESK & CES_AllowRValueReferenceType) || 
!VDType.isRValueReferenceType())
  return false;
VDNonRefType = VDType.getNonReferenceType()
}
if (!VDNonRefType.isObjectType() || VDNonRefType.isVolatileQualified()) 
  return false;
```

or whatever name you find appropriate. Actually it's the type of the 
`DeclRefExpr`, isn't it? So maybe `DREType`?

The initialization might be a bit misleading, an alternative would be to not 
initialize and have an assignment `VDNonRefType = VDType` in the else branch 
instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

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

[PATCH] D95448: [flang][driver] Add support for `-J/-module-dir`

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1006-1007
   MarshallingInfoString>;
+def module_dir : Separate<["-"], "module-dir">,
+  Flags<[FlangOption,FC1Option]>, HelpText<"Add to the list of directories to 
be searched by an USE statement">;
 def dsym_dir : JoinedOrSeparate<["-"], "dsym-dir">,

awarzynski wrote:
> awarzynski wrote:
> > As we are trying to follow `gfortran`, I suggest that we copy the help 
> > message from there:
> > 
> > ```
> > $ gfortran --help=separate | grep '\-J'
> >   -J   Put MODULE files in 'directory'
> > ```
> > Also, we can add the long version (via `DocBrief` field) from here 
> > https://gcc.gnu.org/onlinedocs/gfortran/Directory-Options.html:
> > ```
> > This option specifies where to put .mod files for compiled modules. It is 
> > also added to the list of directories to searched by an USE statement.
> > 
> > The default is the current directory.
> > ```
> > 
> > I appreciate that this patch only implements the 2nd part of what the 
> > option is intended to offer (i.e. updates the search patch for module 
> > files). But I think that it's worthwhile to make the intent behind this 
> > option clear from the very beginning. We can use the commit message to 
> > document the current limitations.
> > 
> > Also, please keep in mind that this help message is going to be re-used by 
> > `-J`, which belongs to `gfortran_Group`. So the description needs to be 
> > valid for both.
> No indentation in `DocBrief`, see this [[ 
> https://github.com/llvm/llvm-project/blob/21bfd068b32ece1c6fbc912208e7cd1782a8c3fc/clang/include/clang/Driver/Options.td#L680-L688
>  | example ]]
> 
> Also, `Put MODULE files in 'directory'` -> `Put MODULE files in ` (the 
> option is displayed as `-module-dir `).
Is the clang file change needed?

`clang -help` now has `  -module-dirPut MODULE files in ` 
while I think the option only makes sense for flang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95448

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

[PATCH] D95007: [CUDA][HIP] Add -fuse-cuid

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Should `-cuid=` be `--cuid=`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95007

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

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

[Ryan Prichard via Phabricator via cfe-commits]

rprichard added a comment.

In D96403#2638883 , @smeenai wrote:

> With NDK r22, I only see libunwind.a for armv7. Will it be provided for other 
> architectures in future NDK versions?

NDK r23 should have a libunwind.a for all architectures. This change isn't in 
the r23 beta 2 that was just released, but it is in the current NDK canary 
build at https://ci.android.com/builds/branches/aosp-master-ndk/grid. (e.g. 
build 7220482 
).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96403

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

hoy wrote:
> dblaikie wrote:
> > hoy wrote:
> > > dblaikie wrote:
> > > > Does this need to be down here? Or would the code be a well exercised 
> > > > if it was up next to the go declaration above?
> > > Yes, it needs to be here. Otherwise it will just like the function `bar` 
> > > above that doesn't get a uniquefied name. I think moving the definition 
> > > up to right after the declaration hides the declaration.
> > Not sure I follow - do you mean that if the go declaration and go 
> > definition were next to each other, this test would (mechanically speaking) 
> > not validate what the patch? Or that it would be less legible, but still 
> > mechanically correct?
> > 
> > I think it would be (assuming it's still mechanically correct) more legible 
> > to put the declaration next to the definition - the comment describes why 
> > the declaration is significant/why the definition is weird, and seeing all 
> > that together would be clearer to me than spreading it out/having to look 
> > further away to see what's going on.
> When the `go` declaration and `go` definition were next to each other, the go 
> function won't get a uniqufied name at all. The declaration will be 
> overwritten by the definition. Only when the declaration is seen by others, 
> such the callsite in `baz`, the declaration makes a difference by having the 
> callsite use a uniqufied name.
> 
> 
Ah! Interesting, good to know. 

Is that worth supporting, I wonder? I guess it falls out for free/without 
significant additional complexity. I worry about the subtlety of the additional 
declaration changing the behavior here... might be a bit surprising/subtle. But 
maybe no nice way to avoid it either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added subscribers: lanza, smeenai.
smeenai added a comment.

With NDK r22, I only see libunwind.a for armv7. Will it be provided for other 
architectures in future NDK versions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96403

___
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#2638609 , @akhuang wrote:

> In D97411#2611142 , @probinson wrote:
>
>> In D97411#2598625 , @akhuang wrote:
>>
>>> I started looking into some diffs of debug info in libc++ tests, but it's 
>>> pretty hard to tell what's different - as far as I can see, there are just 
>>> a bunch of `__hash_value_type`s and `__value_type`s.
>>
>> This is a job for llvm-dva!  See the preliminary patch at D88661 
>> , although it's getting a bit old and might 
>> not apply/build cleanly.
>>
>> (llvm-dva is undergoing an internal review at the moment, we hope to have a 
>> proper reviewable patch series up soon-ish.)
>
> cool, thanks! I tried using it to compare a file that just constructs all of 
> the libc++ types, and it works pretty well (definitely nicer than 
> processing/diffing the dwarf dumps).
>
> One thing I ran into is that I think it'll classify a type as missing/added 
> if it appears in a different order. Not sure if there's a way to make it 
> ignore order, but it wasn't too hard to look through them all.
>
> Anyway, comparing the types did find one class (allocator 
> )
>  that I didn't include the list of types to add the attribute to. Seems like 
> it has some empty inline constructors that are marked constexpr after c++17. 
> Maybe we don't need to have it because it doesn't affect printing the types 
> in the debugger.

Hmm - is that type used in a way that invokes Undefined Behavior? Or is this a 
gap/bug in the ctor homing? I thought there was already a special case for 
constexpr ctors that opted them out of ctor homing.


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] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The recommended named parameter form is probably `/*Param=*/something`. It is 
liked by both 
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-argument-comment.html 
and clang-format.
Another form look to me as well, if both the two tools like it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

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

[PATCH] D97902: [docs] Improve documentation of -B and --gcc-toolchain

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG94a793f09665: [docs] Improve documentation of -B and 
--gcc-toolchain (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97902

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -601,8 +601,14 @@
 def _DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>,
 Flags<[NoXarchOption, CoreOption]>;
 def A : JoinedOrSeparate<["-"], "A">, Flags<[RenderJoined]>, 
Group;
-def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
-HelpText<"Add  to search path for binaries and object files used 
implicitly">;
+def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
+HelpText<"Search $prefix/$triple-$file and $prefix$file for executables, 
libraries, "
+"includes, and data files used by the compiler. $prefix may or may not be 
a directory">;
+def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
+  HelpText<"Search for GCC installation in the specified directory on targets 
which commonly use GCC. "
+  "The directory usually contains 'lib{,32,64}/gcc{,-cross}/$triple' and 
'include'. If specified, "
+  "sysroot is skipped for GCC detection. Note: executables (e.g. ld) used by 
the compiler are not "
+  "overridden by the selected GCC installation">;
 def CC : Flag<["-"], "CC">, Flags<[CC1Option]>, Group,
 HelpText<"Include comments from within macros in preprocessed output">,
 MarshallingInfoFlag>;
@@ -3673,8 +3679,6 @@
   MarshallingInfoFlag>;
 def mcpu_EQ_QUESTION : Flag<["-"], "mcpu=?">, Alias;
 def mtune_EQ_QUESTION : Flag<["-"], "mtune=?">, Alias;
-def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
-  HelpText<"Use the gcc toolchain at the given directory">;
 def time : Flag<["-"], "time">,
   HelpText<"Time individual commands">;
 def traditional_cpp : Flag<["-", "--"], "traditional-cpp">, Flags<[CC1Option]>,
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -18,9 +18,9 @@
 
 
 .. program:: clang
-.. option:: -B, --prefix , --prefix=
+.. option:: -B, --prefix , --prefix=
 
-Add  to search path for binaries and object files used implicitly
+Search $prefix/$triple-$file and $prefix$file for executables, libraries, 
includes, and data files used by the compiler. $prefix may or may not be a 
directory
 
 .. option:: -F
 
@@ -256,7 +256,7 @@
 
 .. option:: --gcc-toolchain=, -gcc-toolchain 
 
-Use the gcc toolchain at the given directory
+Search for GCC installation in the specified directory on targets which 
commonly use GCC. The directory usually contains 
'lib{,32,64}/gcc{,-cross}/$triple' and 'include'. If specified, sysroot is 
skipped for GCC detection. Note: executables (e.g. ld) used by the compiler are 
not overridden by the selected GCC installation
 
 .. option:: -gcodeview
 


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -601,8 +601,14 @@
 def _DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>,
 Flags<[NoXarchOption, CoreOption]>;
 def A : JoinedOrSeparate<["-"], "A">, Flags<[RenderJoined]>, Group;
-def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
-HelpText<"Add  to search path for binaries and object files used implicitly">;
+def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
+HelpText<"Search $prefix/$triple-$file and $prefix$file for executables, libraries, "
+"includes, and data files used by the compiler. $prefix may or may not be a directory">;
+def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
+  HelpText<"Search for GCC installation in the specified directory on targets which commonly use GCC. "
+  "The directory usually contains 'lib{,32,64}/gcc{,-cross}/$triple' and 'include'. If specified, "
+  "sysroot is skipped for GCC detection. Note: executables (e.g. ld) used by the compiler are not "
+  "overridden by the selected GCC installation">;
 def CC : Flag<["-"], "CC">, Flags<[CC1Option]>, Group,
 HelpText<"Include comments from within macros in preprocessed output">,
 MarshallingInfoFlag>;
@@ -3673,8 +3679,6 @@
   MarshallingInfoFlag>;
 def mcpu_EQ_QUESTION : Flag<["-"], "mcpu=?">, Alias;
 def mtune_EQ_QUESTION : Flag<["-"], "mtune=?">, Alias;
-def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
-  HelpText<"Use the gcc toolchain at the given directory">;
 def time : Flag<["-"], "time">,
   

[clang] 94a793f - [docs] Improve documentation of -B and --gcc-toolchain

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-03-19T15:42:37-07:00
New Revision: 94a793f096653fa3536f39c6c1b9e3281907619f

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

LOG: [docs] Improve documentation of -B and --gcc-toolchain

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

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Driver/Options.td

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index bca5722f80d0..962d717483e0 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -18,9 +18,9 @@ GCC-compatible ``clang`` and ``clang++`` drivers.
 
 
 .. program:: clang
-.. option:: -B, --prefix , --prefix=
+.. option:: -B, --prefix , --prefix=
 
-Add  to search path for binaries and object files used implicitly
+Search $prefix/$triple-$file and $prefix$file for executables, libraries, 
includes, and data files used by the compiler. $prefix may or may not be a 
directory
 
 .. option:: -F
 
@@ -256,7 +256,7 @@ Build this module as a system module. Only used with 
-emit-module
 
 .. option:: --gcc-toolchain=, -gcc-toolchain 
 
-Use the gcc toolchain at the given directory
+Search for GCC installation in the specified directory on targets which 
commonly use GCC. The directory usually contains 
'lib{,32,64}/gcc{,-cross}/$triple' and 'include'. If specified, sysroot is 
skipped for GCC detection. Note: executables (e.g. ld) used by the compiler are 
not overridden by the selected GCC installation
 
 .. option:: -gcodeview
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index b7efb7469a23..85a0e02e6357 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -601,8 +601,14 @@ def _HASH_HASH_HASH : Flag<["-"], "###">, 
Flags<[NoXarchOption, CoreOption, Flan
 def _DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>,
 Flags<[NoXarchOption, CoreOption]>;
 def A : JoinedOrSeparate<["-"], "A">, Flags<[RenderJoined]>, 
Group;
-def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
-HelpText<"Add  to search path for binaries and object files used 
implicitly">;
+def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
+HelpText<"Search $prefix/$triple-$file and $prefix$file for executables, 
libraries, "
+"includes, and data files used by the compiler. $prefix may or may not be 
a directory">;
+def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
+  HelpText<"Search for GCC installation in the specified directory on targets 
which commonly use GCC. "
+  "The directory usually contains 'lib{,32,64}/gcc{,-cross}/$triple' and 
'include'. If specified, "
+  "sysroot is skipped for GCC detection. Note: executables (e.g. ld) used by 
the compiler are not "
+  "overridden by the selected GCC installation">;
 def CC : Flag<["-"], "CC">, Flags<[CC1Option]>, Group,
 HelpText<"Include comments from within macros in preprocessed output">,
 MarshallingInfoFlag>;
@@ -3673,8 +3679,6 @@ def print_supported_cpus : Flag<["-", "--"], 
"print-supported-cpus">,
   MarshallingInfoFlag>;
 def mcpu_EQ_QUESTION : Flag<["-"], "mcpu=?">, Alias;
 def mtune_EQ_QUESTION : Flag<["-"], "mtune=?">, Alias;
-def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
-  HelpText<"Use the gcc toolchain at the given directory">;
 def time : Flag<["-"], "time">,
   HelpText<"Time individual commands">;
 def traditional_cpp : Flag<["-", "--"], "traditional-cpp">, Flags<[CC1Option]>,



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

[PATCH] D97993: [Driver] Suppress GCC detection under -B

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4c2da8641087: [Driver] Suppress GCC detection under -B 
(authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97993

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/android-ndk-standalone.cpp
  clang/test/Driver/android-standalone.cpp
  clang/test/Driver/gcc-toolchain.cpp
  clang/test/Driver/print-multi-directory.c

Index: clang/test/Driver/print-multi-directory.c
===
--- clang/test/Driver/print-multi-directory.c
+++ clang/test/Driver/print-multi-directory.c
@@ -19,7 +19,7 @@
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>/dev/null \
 // RUN: -target arm-linux-androideabi21 \
 // RUN: -mthumb \
-// RUN: -B%S/Inputs/basic_android_ndk_tree \
+// RUN: --gcc-toolchain=%S/Inputs/basic_android_ndk_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
 // RUN: -print-multi-directory \
 // RUN:   | FileCheck --match-full-lines --check-prefix=CHECK-ARM-MULTILIBS %s
Index: clang/test/Driver/gcc-toolchain.cpp
===
--- clang/test/Driver/gcc-toolchain.cpp
+++ clang/test/Driver/gcc-toolchain.cpp
@@ -29,3 +29,14 @@
 // CHECK: "{{[^"]*}}/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5{{/|}}crtbegin.o"
 // CHECK: "-L[[TOOLCHAIN]]/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5"
 // CHECK: "-L[[TOOLCHAIN]]/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5/../../../.."
+
+/// Test we don't detect GCC installation under -B.
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN:   --target=aarch64-suse-linux --gcc-toolchain=%S/Inputs/opensuse_42.2_aarch64_tree/usr | \
+// RUN:   FileCheck %s --check-prefix=AARCH64
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN:   --target=aarch64-suse-linux -B%S/Inputs/opensuse_42.2_aarch64_tree/usr | \
+// RUN:   FileCheck %s --check-prefix=NO_AARCH64
+
+// AARCH64:Inputs{{[^"]+}}aarch64-suse-linux/{{[^"]+}}crt1.o"
+// NO_AARCH64-NOT: Inputs{{[^"]+}}aarch64-suse-linux/{{[^"]+}}crt1.o"
Index: clang/test/Driver/android-standalone.cpp
===
--- clang/test/Driver/android-standalone.cpp
+++ clang/test/Driver/android-standalone.cpp
@@ -3,7 +3,7 @@
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi -stdlib=libstdc++ \
-// RUN: -B%S/Inputs/basic_android_tree \
+// RUN: --gcc-toolchain=%S/Inputs/basic_android_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \
 // RUN:   | FileCheck  %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
@@ -18,7 +18,7 @@
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target aarch64-linux-android -stdlib=libstdc++ \
-// RUN: -B%S/Inputs/basic_android_tree \
+// RUN: --gcc-toolchain=%S/Inputs/basic_android_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \
 // RUN:   | FileCheck --check-prefix=CHECK-AARCH64 %s
 // CHECK-AARCH64: {{.*}}clang{{.*}}" "-cc1"
@@ -33,7 +33,7 @@
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm64-linux-android -stdlib=libstdc++ \
-// RUN: -B%S/Inputs/basic_android_tree \
+// RUN: --gcc-toolchain=%S/Inputs/basic_android_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \
 // RUN:   | FileCheck --check-prefix=CHECK-ARM64 %s
 // CHECK-ARM64: {{.*}}clang{{.*}}" "-cc1"
@@ -49,7 +49,7 @@
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target mipsel-linux-android \
 // RUN: -mips32 -stdlib=libstdc++ \
-// RUN: -B%S/Inputs/basic_android_tree \
+// RUN: --gcc-toolchain=%S/Inputs/basic_android_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPS %s
 // CHECK-MIPS: {{.*}}clang{{.*}}" "-cc1"
@@ -65,7 +65,7 @@
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target mipsel-linux-android \
 // RUN: -march=mips32 -mips32r2 -stdlib=libstdc++ \
-// RUN: -B%S/Inputs/basic_android_tree \
+// RUN: --gcc-toolchain=%S/Inputs/basic_android_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPSR2 %s
 // CHECK-MIPSR2: {{.*}}clang{{.*}}" "-cc1"
@@ -81,7 +81,7 @@
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target mipsel-linux-android \
 // RUN: -mips32 -march=mips32r2 -stdlib=libstdc++ \
-// RUN: -B%S/Inputs/basic_android_tree \
+// RUN: --gcc-toolchain=%S/Inputs/basic_android_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPSR2-A %s
 // CHECK-MIPSR2-A: 

[clang] 4c2da86 - [Driver] Suppress GCC detection under -B

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-03-19T15:42:18-07:00
New Revision: 4c2da8641087f7b734337a6e6306329cd2535d60

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

LOG: [Driver] Suppress GCC detection under -B

In GCC, if `-B $prefix` is specified, `$prefix` is used to find executable 
files and startup files.
`$prefix/include` is added as an include search directory.

Clang overloads -B with GCC installation detection semantics which make the
behavior less predictable (due to the "largest GCC version wins" rule) and
interact poorly with --gcc-toolchain (--gcc-toolchain can be overridden by -B).

* `clang++ foo.cpp` detects GCC installation under `/usr`.
* `clang++ --gcc-toolchain=Inputs foo.cpp` detects GCC installation under 
`Inputs`.
* `clang++ -BA --gcc-toolchain=B foo.cpp` detects GCC installation under A and 
B and the larger version wins. With this patch, only B is used for detection.
* `clang++ -BA foo.cpp` detects GCC installation under `A` and `/usr`, and the 
larger GCC version wins. With this patch `A` is not used for detection.

This patch changes -B to drop the GCC detection semantics.  Its executable
searching semantics are preserved.  --gcc-toolchain is the recommended option to
specify the GCC installation detection directory.

(
Note: Clang detects GCC installation in various target dependent directories.
`$sysroot/usr` (sysroot defaults to "") is a common directory used by most 
targets.
Such a directory is expected to contain something like 
`lib{,32,64}/gcc{,-cross}/$triple`.
Clang will then construct library/include paths from the directory.
)

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Driver/ToolChains/Gnu.cpp
clang/test/Driver/android-ndk-standalone.cpp
clang/test/Driver/android-standalone.cpp
clang/test/Driver/gcc-toolchain.cpp
clang/test/Driver/print-multi-directory.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c78445b9be6f..d4c9f53b82c0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -72,6 +72,13 @@ Modified Compiler Flags
 ---
 
 - -Wshadow now also checks for shadowed structured bindings
+- ``-B `` (when  is a directory) was overloaded to 
additionally
+  detect GCC installations under  
(``lib{,32,64}/gcc{,-cross}/$triple``).
+  This behavior was incompatible with GCC, caused interop issues with
+  ``--gcc-toolchain``, and was thus dropped. Specify ``--gcc-toolchain=``
+  instead. ``-B``'s other GCC-compatible semantics are preserved:
+  ``$prefix/$triple-$file`` and ``$prefix$file`` are searched for executables,
+  libraries, includes, and data files used by the compiler.
 
 Removed Compiler Flags
 -

diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index fbf2f29e0514..38971288e38f 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1909,9 +1909,7 @@ void Generic_GCC::GCCInstallationDetector::init(
CandidateBiarchTripleAliases);
 
   // Compute the set of prefixes for our search.
-  SmallVector Prefixes(D.PrefixDirs.begin(),
-   D.PrefixDirs.end());
-
+  SmallVector Prefixes;
   StringRef GCCToolchainDir = getGCCToolchainDir(Args, D.SysRoot);
   if (GCCToolchainDir != "") {
 if (GCCToolchainDir.back() == '/')

diff  --git a/clang/test/Driver/android-ndk-standalone.cpp 
b/clang/test/Driver/android-ndk-standalone.cpp
index c4d939934782..8581963ae00d 100644
--- a/clang/test/Driver/android-ndk-standalone.cpp
+++ b/clang/test/Driver/android-ndk-standalone.cpp
@@ -3,7 +3,7 @@
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi21 \
-// RUN: -B%S/Inputs/basic_android_ndk_tree \
+// RUN: --gcc-toolchain=%S/Inputs/basic_android_ndk_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
 // RUN:   | FileCheck  %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
@@ -34,7 +34,7 @@
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi14 \
-// RUN: -B%S/Inputs/basic_android_ndk_tree \
+// RUN: --gcc-toolchain=%S/Inputs/basic_android_ndk_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
 // RUN:   | FileCheck --check-prefix=CHECK-14 %s
 // CHECK-14: "-L{{.*}}/sysroot/usr/lib/arm-linux-androideabi/14"
@@ -42,7 +42,7 @@
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \
-// RUN: -B%S/Inputs/basic_android_ndk_tree \
+// RUN: 

[PATCH] D98948: [analyzer][solver] Fix infeasible constraints (PR49642)

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Aha ok, seems reasonable, thx!




Comment at: clang/test/Analysis/PR49642.c:1
+// RUN: %clang --analyze -Xclang -analyzer-checker=core %s
+

Why not the usual `%clang_analyze_cc1`? Your approach only adds driver testing 
which doesn't seem to test anything new. Also if you do `clang --analyze` you 
don't need to enable `core` explicitly, you already have all on-by-default 
checks running.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98948

___
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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.

Thank you!




Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:390-391
+
+if (!TM.getTargetTriple().isArch64Bit())
+  return false;
+

gulfem wrote:
> lebedev.ri wrote:
> > gulfem wrote:
> > > lebedev.ri wrote:
> > > > 1. But all tests are using `x86_64` triple?
> > > > 2. This is somewhat backwards. if the target wants to disable this, it 
> > > > will need to override this function with `return false;`.
> > > 1. Although I used `x86_64 triple`, this optimization can be applied to 
> > > other 64-bit architectures too, because it not target dependent except 
> > > `isArch64Bit` and `getCodeModel` check.
> > > 2. Is there a target that you have in mind that we need to disable this 
> > > optimization? 
> > > I thought that it makes sense to enable this optimization by default on 
> > > all the targets that can support it.
> > > In case targets want to disable it, they can override it as you said.
> > > How can we improve the implementation?
> > > If you have suggestions, I'm happy to incorporate that.
> > > 
> > I'm sorry, i do not understand.
> > Why does `!TM.getTargetTriple().isArch64Bit()` check exist?
> > To me it reads as "if we aren't compiling for AArch64, don't build rel 
> > lookup tables".
> > Am i misreading this?
> `isArch64Bit` checks whether we have a 64-bit architecture, right?
> I don't think it specifically checks for `AArch64`, and it can cover other 
> 64-bit architectures like `x86_64` as well.
> isArch64Bit checks whether we have a 64-bit architecture, right?

D'oh. I really did read it as A*A*rch64 :/ Sorry.



Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:172-173
+
+  for (auto GVI = M.global_begin(), E = M.global_end(); GVI != E;) {
+GlobalVariable  = *GVI++;
+

`make_early_inc_range()`?



Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:200
+
+  return PreservedAnalyses::none();
+}

I would think this should be
```
PreservedAnalyses PA;
PA.preserveSet();
return PA;
```
since this doesn't touch CFG at all.
I think this should get rid of redundant `Running analysis: TargetIRAnalysis`.


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 inline comments.



Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:390-391
+
+if (!TM.getTargetTriple().isArch64Bit())
+  return false;
+

lebedev.ri wrote:
> gulfem wrote:
> > lebedev.ri wrote:
> > > 1. But all tests are using `x86_64` triple?
> > > 2. This is somewhat backwards. if the target wants to disable this, it 
> > > will need to override this function with `return false;`.
> > 1. Although I used `x86_64 triple`, this optimization can be applied to 
> > other 64-bit architectures too, because it not target dependent except 
> > `isArch64Bit` and `getCodeModel` check.
> > 2. Is there a target that you have in mind that we need to disable this 
> > optimization? 
> > I thought that it makes sense to enable this optimization by default on all 
> > the targets that can support it.
> > In case targets want to disable it, they can override it as you said.
> > How can we improve the implementation?
> > If you have suggestions, I'm happy to incorporate that.
> > 
> I'm sorry, i do not understand.
> Why does `!TM.getTargetTriple().isArch64Bit()` check exist?
> To me it reads as "if we aren't compiling for AArch64, don't build rel lookup 
> tables".
> Am i misreading this?
`isArch64Bit` checks whether we have a 64-bit architecture, right?
I don't think it specifically checks for `AArch64`, and it can cover other 
64-bit architectures like `x86_64` as well.


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] 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 added a comment.

In D97411#2611142 , @probinson wrote:

> In D97411#2598625 , @akhuang wrote:
>
>> I started looking into some diffs of debug info in libc++ tests, but it's 
>> pretty hard to tell what's different - as far as I can see, there are just a 
>> bunch of `__hash_value_type`s and `__value_type`s.
>
> This is a job for llvm-dva!  See the preliminary patch at D88661 
> , although it's getting a bit old and might 
> not apply/build cleanly.
>
> (llvm-dva is undergoing an internal review at the moment, we hope to have a 
> proper reviewable patch series up soon-ish.)

cool, thanks! I tried using it to compare a file that just constructs all of 
the libc++ types, and it works pretty well (definitely nicer than 
processing/diffing the dwarf dumps).

One thing I ran into is that I think it'll classify a type as missing/added if 
it appears in a different order. Not sure if there's a way to make it ignore 
order, but it wasn't too hard to look through them all.

Anyway, comparing the types did find one class (allocator 
)
 that I didn't include the list of types to add the attribute to. Seems like it 
has some empty inline constructors that are marked constexpr after c++17. Maybe 
we don't need to have it because it doesn't affect printing the types in the 
debugger.


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] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer added a comment.

Invalid IR generation should be addressed by 
19d2c65ddd757997785163709800f837857f686d 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98816

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

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

Quuxplusone wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > mizvekov wrote:
> > > > A drive by fix here would be that we already have a VDType in this 
> > > > context, might as well use it even though original for some reason 
> > > > missed it in this part.
> > > This whole block is also logically equivalent to the much simpler:
> > > ```
> > > if (VDType.isReferenceType()) {
> > > if (!(CESK & CES_AllowRValueReferenceType) || 
> > > !VDType.isRValueReferenceType())
> > >   return false;
> > > VDType = VDType.getNonReferenceType()
> > > }
> > > if (!VDType.isObjectType() || VDType.isVolatileQualified()) 
> > >   return false;
> > > ```
> > > But you do have to adjust the comments there and adjust the rest to use 
> > > VDType consistently :)
> > > Also, I think it might be possible to even remove the 
> > > `!VDType.isObjectType() || ` part from my suggestion above, because it 
> > > might be the only option left if it is not a reference anyway.
> > ```
> >   bool isObjectType() const {
> > // C++ [basic.types]p8:
> > //   An object type is a (possibly cv-qualified) type that is not a
> > //   function type, not a reference type, and not a void type.
> > return !isReferenceType() && !isFunctionType() && !isVoidType();
> >   }
> > ```
> > So yeah I think you can just make my suggestion be:
> > ```
> > if (VDType.isReferenceType()) {
> > if (!(CESK & CES_AllowRValueReferenceType) || 
> > !VDType.isRValueReferenceType())
> >   return false;
> > VDType = VDType.getNonReferenceType()
> > }
> > if (VDType.isVolatileQualified()) 
> >   return false;
> > ```
> > 
> > 
> Yeah but I //reaally// don't want to
> (1) change the meaning of `VDType` in the middle of this function (mantra: 
> "one name = one meaning")
> (2) get "clever". I don't want to have to think about "Are there any types 
> that are neither object types nor reference types?" (What about function 
> types? What about block types? What about, I dunno, bit-fields?) I want the 
> code to be //obviously correct//, and also to casewise match exactly what the 
> standard says. It says object or rvalue reference type — let's write code 
> that expresses that wording //exactly//.
How about:
```
QualType VDObjType = VDType;
if (!VDType.isObjectType()) {
if (!(CESK & CES_AllowRValueReferenceType) || 
!VDType.isRValueReferenceType())
  return false;
VDObjType = VDType.getNonReferenceType();
}
if (VDObjType .isVolatileQualified()) 
  return false;
```
And then `s/VDType/VDObjType/` from here on.
I think this expresses the meaning of the standard clearly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

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

[PATCH] D97371: [clang][parser] Remove questionable ProhibitAttributes() call in objc parsing

[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, thanks for seeing this through


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

https://reviews.llvm.org/D97371

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

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone marked 2 inline comments as done.
Quuxplusone added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

mizvekov wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > A drive by fix here would be that we already have a VDType in this 
> > > context, might as well use it even though original for some reason missed 
> > > it in this part.
> > This whole block is also logically equivalent to the much simpler:
> > ```
> > if (VDType.isReferenceType()) {
> > if (!(CESK & CES_AllowRValueReferenceType) || 
> > !VDType.isRValueReferenceType())
> >   return false;
> > VDType = VDType.getNonReferenceType()
> > }
> > if (!VDType.isObjectType() || VDType.isVolatileQualified()) 
> >   return false;
> > ```
> > But you do have to adjust the comments there and adjust the rest to use 
> > VDType consistently :)
> > Also, I think it might be possible to even remove the 
> > `!VDType.isObjectType() || ` part from my suggestion above, because it 
> > might be the only option left if it is not a reference anyway.
> ```
>   bool isObjectType() const {
> // C++ [basic.types]p8:
> //   An object type is a (possibly cv-qualified) type that is not a
> //   function type, not a reference type, and not a void type.
> return !isReferenceType() && !isFunctionType() && !isVoidType();
>   }
> ```
> So yeah I think you can just make my suggestion be:
> ```
> if (VDType.isReferenceType()) {
> if (!(CESK & CES_AllowRValueReferenceType) || 
> !VDType.isRValueReferenceType())
>   return false;
> VDType = VDType.getNonReferenceType()
> }
> if (VDType.isVolatileQualified()) 
>   return false;
> ```
> 
> 
Yeah but I //reaally// don't want to
(1) change the meaning of `VDType` in the middle of this function (mantra: "one 
name = one meaning")
(2) get "clever". I don't want to have to think about "Are there any types that 
are neither object types nor reference types?" (What about function types? What 
about block types? What about, I dunno, bit-fields?) I want the code to be 
//obviously correct//, and also to casewise match exactly what the standard 
says. It says object or rvalue reference type — let's write code that expresses 
that wording //exactly//.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

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

[PATCH] D98980: [CodeGen] Don't crash on for loops with cond variables and no increment

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Yes, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98980

___
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 added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:298
+  driverPath.append("/../include/flang/");
+  return driverPath.str().str();
+}

awarzynski wrote:
> Given this [[ 
> https://github.com/llvm/llvm-project/blob/987ee6e3cc1fb672b3ed201e72a5281c2ec88c99/llvm/include/llvm/ADT/SmallString.h#L271-L273
>  | conversion operator ]], wouldn't `return std::string(driverPath);` be more 
> efficient? 
Done, thanks


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

https://reviews.llvm.org/D97080

___
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 331984.
arnamoy10 added a comment.

Addressing reviewers' comments


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/Driver/Inputs/ieee_arithmetic.mod
  flang/test/Driver/Inputs/iso_fortran_env.mod
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/intrinsic_module_path.f90

Index: flang/test/Driver/intrinsic_module_path.f90
===
--- /dev/null
+++ flang/test/Driver/intrinsic_module_path.f90
@@ -0,0 +1,37 @@
+! Ensure argument -fintrinsic-modules-path works as expected.
+! WITHOUT the option, the default location for the module is checked and no error generated.
+! With the option GIVEN, the module with the same name is PREPENDED, and considered over the
+! default one, causing a CHECKSUM error.
+
+! REQUIRES: new-flang-driver
+
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: %flang-new -fsyntax-only %s  2>&1 | FileCheck %s --allow-empty --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: %flang-new -fc1 %s  2>&1 | FileCheck %s --allow-empty --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-NOT: 'ieee_arithmetic.mod' was not found
+! WITHOUT-NOT: 'iso_fortran_env.mod' was not found
+
+!-
+! EXPECTED OUTPUT WITH
+!-
+! GIVEN: error: Cannot read module file for module 'ieee_arithmetic': File has invalid checksum
+! GIVEN: error: Cannot read module file for module 'iso_fortran_env': File has invalid checksum
+
+
+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/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -35,6 +35,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: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
@@ -83,6 +85,8 @@
 ! HELP-FC1-NEXT: -fget-symbols-sources   Dump symbols and their source code locations
 ! 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: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-FC1-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-FC1-NEXT: -fopenacc  Enable OpenACC
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -35,6 +35,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: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! CHECK-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! CHECK-NEXT: -fno-color-diagnostics Disable colors in diagnostics
Index: flang/test/Driver/Inputs/iso_fortran_env.mod
===
--- /dev/null
+++ flang/test/Driver/Inputs/iso_fortran_env.mod

[PATCH] D98980: [CodeGen] Don't crash on for loops with cond variables and no increment

[Benjamin Kramer 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 rG19d2c65ddd75: [CodeGen] Dont crash on for loops with 
cond variables and no increment (authored by bkramer).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98980

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGenCXX/for-cond-var.cpp


Index: clang/test/CodeGenCXX/for-cond-var.cpp
===
--- clang/test/CodeGenCXX/for-cond-var.cpp
+++ clang/test/CodeGenCXX/for-cond-var.cpp
@@ -123,3 +123,16 @@
   // CHECK [[for_end]]:
   // CHECK: ret void
 }
+
+// CHECK: define {{.*}} void @_Z16incless_for_loopv(
+void incless_for_loop() {
+  // CHECK: br label %[[for_cond:.*]]
+  // CHECK: [[for_cond]]:
+  // CHECK:   br i1 {{.*}}, label %[[for_body:.*]], label %[[for_end:.*]]
+  // CHECK: [[for_body]]:
+  // CHECK:   br label %[[for_cond]]
+  // CHECK: [[for_end]]:
+  // CHECK:   ret void
+  // CHECK: }
+  for (; int b = 0;) continue;
+}
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -992,7 +992,7 @@
 
   // We have entered the condition variable's scope, so we're now able to
   // jump to the continue block.
-  Continue = getJumpDestInCurrentScope("for.inc");
+  Continue = S.getInc() ? getJumpDestInCurrentScope("for.inc") : CondDest;
   BreakContinueStack.back().ContinueBlock = Continue;
 }
 


Index: clang/test/CodeGenCXX/for-cond-var.cpp
===
--- clang/test/CodeGenCXX/for-cond-var.cpp
+++ clang/test/CodeGenCXX/for-cond-var.cpp
@@ -123,3 +123,16 @@
   // CHECK [[for_end]]:
   // CHECK: ret void
 }
+
+// CHECK: define {{.*}} void @_Z16incless_for_loopv(
+void incless_for_loop() {
+  // CHECK: br label %[[for_cond:.*]]
+  // CHECK: [[for_cond]]:
+  // CHECK:   br i1 {{.*}}, label %[[for_body:.*]], label %[[for_end:.*]]
+  // CHECK: [[for_body]]:
+  // CHECK:   br label %[[for_cond]]
+  // CHECK: [[for_end]]:
+  // CHECK:   ret void
+  // CHECK: }
+  for (; int b = 0;) continue;
+}
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -992,7 +992,7 @@
 
   // We have entered the condition variable's scope, so we're now able to
   // jump to the continue block.
-  Continue = getJumpDestInCurrentScope("for.inc");
+  Continue = S.getInc() ? getJumpDestInCurrentScope("for.inc") : CondDest;
   BreakContinueStack.back().ContinueBlock = Continue;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 19d2c65 - [CodeGen] Don't crash on for loops with cond variables and no increment

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2021-03-19T20:43:52+01:00
New Revision: 19d2c65ddd757997785163709800f837857f686d

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

LOG: [CodeGen] Don't crash on for loops with cond variables and no increment

This looks like an oversight from a875721d8a2d, creating IR that refers
to `for.inc` even if it doesn't exist.

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

Added: 


Modified: 
clang/lib/CodeGen/CGStmt.cpp
clang/test/CodeGenCXX/for-cond-var.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 6461e2011216..38f3aa941415 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -992,7 +992,7 @@ void CodeGenFunction::EmitForStmt(const ForStmt ,
 
   // We have entered the condition variable's scope, so we're now able to
   // jump to the continue block.
-  Continue = getJumpDestInCurrentScope("for.inc");
+  Continue = S.getInc() ? getJumpDestInCurrentScope("for.inc") : CondDest;
   BreakContinueStack.back().ContinueBlock = Continue;
 }
 

diff  --git a/clang/test/CodeGenCXX/for-cond-var.cpp 
b/clang/test/CodeGenCXX/for-cond-var.cpp
index 45b4a82cb905..60e54d4141f7 100644
--- a/clang/test/CodeGenCXX/for-cond-var.cpp
+++ b/clang/test/CodeGenCXX/for-cond-var.cpp
@@ -123,3 +123,16 @@ void PR49585_break() {
   // CHECK [[for_end]]:
   // CHECK: ret void
 }
+
+// CHECK: define {{.*}} void @_Z16incless_for_loopv(
+void incless_for_loop() {
+  // CHECK: br label %[[for_cond:.*]]
+  // CHECK: [[for_cond]]:
+  // CHECK:   br i1 {{.*}}, label %[[for_body:.*]], label %[[for_end:.*]]
+  // CHECK: [[for_body]]:
+  // CHECK:   br label %[[for_cond]]
+  // CHECK: [[for_end]]:
+  // CHECK:   ret void
+  // CHECK: }
+  for (; int b = 0;) continue;
+}



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

[PATCH] D98980: [CodeGen] Don't crash on for loops with cond variables and no increment

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer added a comment.

Landing this as it's a pretty bad crasher. Feel free to point out all my 
mistakes in post-commit review :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98980

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

[PATCH] D98980: [CodeGen] Don't crash on for loops with cond variables and no increment

[Jordan Rupprecht via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98980

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

[PATCH] D98980: [CodeGen] Don't crash on for loops with cond variables and no increment

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer created this revision.
bkramer added reviewers: rupprecht, rjmccall, rsmith.
bkramer requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This looks like an oversight from a875721d8a2d 
, creating 
IR that refers
to `for.inc` even if it doesn't exist.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98980

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGenCXX/for-cond-var.cpp


Index: clang/test/CodeGenCXX/for-cond-var.cpp
===
--- clang/test/CodeGenCXX/for-cond-var.cpp
+++ clang/test/CodeGenCXX/for-cond-var.cpp
@@ -123,3 +123,16 @@
   // CHECK [[for_end]]:
   // CHECK: ret void
 }
+
+// CHECK: define {{.*}} void @_Z16incless_for_loopv(
+void incless_for_loop() {
+  // CHECK: br label %[[for_cond:.*]]
+  // CHECK: [[for_cond]]:
+  // CHECK:   br i1 {{.*}}, label %[[for_body:.*]], label %[[for_end:.*]]
+  // CHECK: [[for_body]]:
+  // CHECK:   br label %[[for_cond]]
+  // CHECK: [[for_end]]:
+  // CHECK:   ret void
+  // CHECK: }
+  for (; int b = 0;) continue;
+}
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -992,7 +992,7 @@
 
   // We have entered the condition variable's scope, so we're now able to
   // jump to the continue block.
-  Continue = getJumpDestInCurrentScope("for.inc");
+  Continue = S.getInc() ? getJumpDestInCurrentScope("for.inc") : CondDest;
   BreakContinueStack.back().ContinueBlock = Continue;
 }
 


Index: clang/test/CodeGenCXX/for-cond-var.cpp
===
--- clang/test/CodeGenCXX/for-cond-var.cpp
+++ clang/test/CodeGenCXX/for-cond-var.cpp
@@ -123,3 +123,16 @@
   // CHECK [[for_end]]:
   // CHECK: ret void
 }
+
+// CHECK: define {{.*}} void @_Z16incless_for_loopv(
+void incless_for_loop() {
+  // CHECK: br label %[[for_cond:.*]]
+  // CHECK: [[for_cond]]:
+  // CHECK:   br i1 {{.*}}, label %[[for_body:.*]], label %[[for_end:.*]]
+  // CHECK: [[for_body]]:
+  // CHECK:   br label %[[for_cond]]
+  // CHECK: [[for_end]]:
+  // CHECK:   ret void
+  // CHECK: }
+  for (; int b = 0;) continue;
+}
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -992,7 +992,7 @@
 
   // We have entered the condition variable's scope, so we're now able to
   // jump to the continue block.
-  Continue = getJumpDestInCurrentScope("for.inc");
+  Continue = S.getInc() ? getJumpDestInCurrentScope("for.inc") : CondDest;
   BreakContinueStack.back().ContinueBlock = Continue;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

mizvekov wrote:
> mizvekov wrote:
> > A drive by fix here would be that we already have a VDType in this context, 
> > might as well use it even though original for some reason missed it in this 
> > part.
> This whole block is also logically equivalent to the much simpler:
> ```
> if (VDType.isReferenceType()) {
> if (!(CESK & CES_AllowRValueReferenceType) || 
> !VDType.isRValueReferenceType())
>   return false;
> VDType = VDType.getNonReferenceType()
> }
> if (!VDType.isObjectType() || VDType.isVolatileQualified()) 
>   return false;
> ```
> But you do have to adjust the comments there and adjust the rest to use 
> VDType consistently :)
> Also, I think it might be possible to even remove the `!VDType.isObjectType() 
> || ` part from my suggestion above, because it might be the only option left 
> if it is not a reference anyway.
```
  bool isObjectType() const {
// C++ [basic.types]p8:
//   An object type is a (possibly cv-qualified) type that is not a
//   function type, not a reference type, and not a void type.
return !isReferenceType() && !isFunctionType() && !isVoidType();
  }
```
So yeah I think you can just make my suggestion be:
```
if (VDType.isReferenceType()) {
if (!(CESK & CES_AllowRValueReferenceType) || 
!VDType.isRValueReferenceType())
  return false;
VDType = VDType.getNonReferenceType()
}
if (VDType.isVolatileQualified()) 
  return false;
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

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

[PATCH] D97119: [flang][driver] Add options for -std=f2018

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/test/Driver/std2018.f90:1
+! REQUIRES: new-flang-driver
+! Ensure argument -std=f2018 works as expected.

arnamoy10 wrote:
> awarzynski wrote:
> > Would it be possible to share this test with `f18`?
> Would you please elaborate on how do want the sharing to happen (same test 
> file/different test file)?  I think in a previous comment you mentioned that 
> it is sufficient to test the frontend driver for this patch?
This line means that the test is not run when `FLANG_BUILD_NEW_DRIVER` is `Off`:
```
! REQUIRES: new-flang-driver
```
Once this line is removed, the test is effectively shared.

Currently this test won't work with `f18`(i.e. when you remove this line), but 
is should be sufficient to add `-pedantic` and `-std=f2018` as aliases for [[ 
https://github.com/llvm/llvm-project/blob/987ee6e3cc1fb672b3ed201e72a5281c2ec88c99/flang/tools/f18/f18.cpp#L489-L491
 | Mstandard ]].


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

https://reviews.llvm.org/D97119

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

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

mizvekov wrote:
> A drive by fix here would be that we already have a VDType in this context, 
> might as well use it even though original for some reason missed it in this 
> part.
This whole block is also logically equivalent to the much simpler:
```
if (VDType.isReferenceType()) {
if (!(CESK & CES_AllowRValueReferenceType) || 
!VDType.isRValueReferenceType())
  return false;
VDType = VDType.getNonReferenceType()
}
if (!VDType.isObjectType() || VDType.isVolatileQualified()) 
  return false;
```
But you do have to adjust the comments there and adjust the rest to use VDType 
consistently :)
Also, I think it might be possible to even remove the `!VDType.isObjectType() 
|| ` part from my suggestion above, because it might be the only option left if 
it is not a reference anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

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

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 331973.
Quuxplusone added a comment.

Per @mizvekov, use `VDType` instead of `VD->getType()` wherever possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp

Index: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
===
--- clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
+++ clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
@@ -292,3 +292,108 @@
   return b; // cxx20-error {{calling a private constructor of class 'test_ctor_param_rvalue_ref::B2'}}
 }
 } // namespace test_ctor_param_rvalue_ref
+
+namespace test_lvalue_ref_is_not_moved_from {
+
+struct Target {};
+  // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable}}
+  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable}}
+  // cxx11_14_17-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+  // cxx11_14_17-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+struct CopyOnly {
+  CopyOnly(CopyOnly&&) = delete; // cxx20-note {{has been explicitly marked deleted here}}
+  CopyOnly(CopyOnly&);
+  operator Target() && = delete; // cxx20-note {{has been explicitly marked deleted here}}
+  operator Target() &;
+};
+
+struct MoveOnly {
+  MoveOnly(MoveOnly&&); // expected-note {{copy constructor is implicitly deleted because}}
+// cxx11_14_17-note@-1 {{copy constructor is implicitly deleted because}}
+  operator Target() &&; // expected-note {{candidate function not viable}}
+// cxx11_14_17-note@-1 {{candidate function not viable}}
+};
+
+extern CopyOnly copyonly;
+extern MoveOnly moveonly;
+
+CopyOnly t1() {
+CopyOnly& r = copyonly;
+return r;
+}
+
+CopyOnly t2() {
+CopyOnly&& r = static_cast(copyonly);
+return r; // cxx20-error {{call to deleted constructor}}
+}
+
+MoveOnly t3() {
+MoveOnly& r = moveonly;
+return r; // expected-error {{call to implicitly-deleted copy constructor}}
+}
+
+MoveOnly t4() {
+MoveOnly&& r = static_cast(moveonly);
+return r; // cxx11_14_17-error {{call to implicitly-deleted copy constructor}}
+}
+
+Target t5() {
+CopyOnly& r = copyonly;
+return r;
+}
+
+Target t6() {
+CopyOnly&& r = static_cast(copyonly);
+return r; // cxx20-error {{invokes a deleted function}}
+}
+
+Target t7() {
+MoveOnly& r = moveonly;
+return r; // expected-error {{no viable conversion}}
+}
+
+Target t8() {
+MoveOnly&& r = static_cast(moveonly);
+return r; // cxx11_14_17-error {{no viable conversion}}
+}
+
+} // namespace test_lvalue_ref_is_not_moved_from
+
+namespace test_rvalue_ref_to_nonobject {
+
+struct CopyOnly {};
+struct MoveOnly {};
+
+struct Target {
+Target(CopyOnly (&)());
+Target(CopyOnly (&&)()) = delete;
+Target(MoveOnly (&)()) = delete; // expected-note {{has been explicitly marked deleted here}}
+  // expected-note@-1 {{has been explicitly marked deleted here}}
+Target(MoveOnly (&&)());
+};
+
+CopyOnly make_copyonly();
+MoveOnly make_moveonly();
+
+Target t1() {
+CopyOnly ()() = make_copyonly;
+return r;
+}
+
+Target t2() {
+CopyOnly (&)() = static_cast(make_copyonly);
+return r; // OK in all modes; not subject to implicit move
+}
+
+Target t3() {
+MoveOnly ()() = make_moveonly;
+return r; // expected-error {{invokes a deleted function}}
+}
+
+Target t4() {
+MoveOnly (&)() = static_cast(make_moveonly);
+return r; // expected-error {{invokes a deleted function}}
+}
+
+} // namespace test_rvalue_ref_to_nonobject
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3092,24 +3092,31 @@
   if (VD->hasAttr())
 return false;
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VDType->isObjectType()) {
+// C++17 [class.copy.elision]p3:
+// ...non-volatile automatic object...
+if (VDType.isVolatileQualified())
+  return false;
+  } else if (VDType->isRValueReferenceType()) {
+// C++20 [class.copy.elision]p3:
+// ...either a non-volatile object or an rvalue reference to a non-volatile object type...
+if (!(CESK & CES_AllowRValueReferenceType))
+  return false;
+if (!VDType.getNonReferenceType()->isObjectType())
+  return false;
+if (VDType.getNonReferenceType().isVolatileQualified())
+  return false;
+  } else {
 return false;
+  }
 
 

[PATCH] D97119: [flang][driver] Add options for -std=f2018

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 added inline comments.



Comment at: flang/test/Driver/std2018.f90:1
+! REQUIRES: new-flang-driver
+! Ensure argument -std=f2018 works as expected.

awarzynski wrote:
> Would it be possible to share this test with `f18`?
Would you please elaborate on how do want the sharing to happen (same test 
file/different test file)?  I think in a previous comment you mentioned that it 
is sufficient to test the frontend driver for this patch?



Comment at: flang/test/Driver/std2018_wrong.f90:14-22
+subroutine foo2()
+do 01 m=1,2
+  select case (m)
+  case default
+print*, "default", m
+  case (1)
+print*, "start"

awarzynski wrote:
> No input is needed for this test, is it? IIUC, this bit can be deleted to 
> make the test leaner.
Sure, will do.


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

https://reviews.llvm.org/D97119

___
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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:390-391
+
+if (!TM.getTargetTriple().isArch64Bit())
+  return false;
+

gulfem wrote:
> lebedev.ri wrote:
> > 1. But all tests are using `x86_64` triple?
> > 2. This is somewhat backwards. if the target wants to disable this, it will 
> > need to override this function with `return false;`.
> 1. Although I used `x86_64 triple`, this optimization can be applied to other 
> 64-bit architectures too, because it not target dependent except 
> `isArch64Bit` and `getCodeModel` check.
> 2. Is there a target that you have in mind that we need to disable this 
> optimization? 
> I thought that it makes sense to enable this optimization by default on all 
> the targets that can support it.
> In case targets want to disable it, they can override it as you said.
> How can we improve the implementation?
> If you have suggestions, I'm happy to incorporate that.
> 
I'm sorry, i do not understand.
Why does `!TM.getTargetTriple().isArch64Bit()` check exist?
To me it reads as "if we aren't compiling for AArch64, don't build rel lookup 
tables".
Am i misreading this?


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] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

I tried creating an IR repro, but: running `-S -emit-llvm` with the old PM 
crashes before it can generate anything, and running with the new PM seems to 
generate invalid IR (branches to %for.inc, which does not exist). I suspect 
this is not an optimizer bug, but crashing in the optimizer because invalid IR 
has been fed to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98816

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

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

A drive by fix here would be that we already have a VDType in this context, 
might as well use it even though original for some reason missed it in this 
part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

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

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

The error message I posted earlier was when using O1 
 + new PM, but it crashes with the 
old one & no optimizations:

  $ cat /tmp/repro.cc
  void a() {
for (; int b = 0;) continue;
  }
  $ clang -c /tmp/repro.cc -o /tmp/repro.o
  Referring to a basic block in another function!
br label %for.inc
  in function _Z1av
  fatal error: error in backend: Broken function found, compilation aborted!
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.  Program arguments: clang -c /tmp/repro.cc -o /tmp/repro.o
  1.   parser at end of file
  2.  Per-function optimization
  3.  Running pass 'Module Verifier' on function '@_Z1av'
  ...
  $ clang -fno-legacy-pass-manager -O1 -c /tmp/repro.cc -o /tmp/repro.o
  clang: 
/home/rupprecht/src/llvm-project/llvm/include/llvm/Support/Casting.h:104: 
static bool llvm::isa_impl_cl::doit(const From *) [To = llvm::InvokeInst, From = const llvm::Instruction 
*]: Assertion `Val && "isa<> used on a null pointer"' failed.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.  Program arguments: bin/clang -fno-legacy-pass-manager -O1 -c 
/tmp/repro.cc -o /tmp/repro.o
  1.   parser at end of file
  2.  Optimizer
  ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98816

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

[PATCH] D98935: [WoA][MSVC] Use default linker setting in MSVC-compatible driver [take 2]

[Maxim Kuvyrkov via Phabricator via cfe-commits]

maxim-kuvyrkov added a subscriber: tstellar.
maxim-kuvyrkov added a comment.

@tstellar

Hi Tom,

This has passed buildbots, so I'll cherry-pick this to release/12.x to fix 
https://bugs.llvm.org/show_bug.cgi?id=49624


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98935

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

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 331955.
nick added a comment.

Forgot to remove a duplicated test


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

https://reviews.llvm.org/D69218

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -297,6 +297,17 @@
   EXPECT_TRUE(matches("int b[7];", M));
 }
 
+TEST_F(RegistryTest, CXXBaseSpecifier) {
+  // TODO: rewrite with top-level cxxBaseSpecifier matcher when available
+  DeclarationMatcher ClassHasAnyDirectBase =
+  constructMatcher("cxxRecordDecl",
+   constructMatcher("hasDirectBase",
+constructMatcher("cxxBaseSpecifier")))
+  .getTypedMatcher();
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+}
+
 TEST_F(RegistryTest, CXXCtorInitializer) {
   Matcher CtorDecl = constructMatcher(
   "cxxConstructorDecl",
Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -320,6 +320,15 @@
 varDecl(hasType(pointsTo(ClassX);
 }
 
+TEST(HasType, TakesQualTypeMatcherAndMatchesCXXBaseSpecifier) {
+  TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, TakesDeclMatcherAndMatchesExpr) {
   DeclarationMatcher ClassX = recordDecl(hasName("X"));
   EXPECT_TRUE(
@@ -337,6 +346,15 @@
 notMatches("class X {}; void y() { X *x; }", varDecl(hasType(ClassX;
 }
 
+TEST(HasType, TakesDeclMatcherAndMatchesCXXBaseSpecifier) {
+  DeclarationMatcher ClassX = recordDecl(hasName("X"));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, MatchesTypedefDecl) {
   EXPECT_TRUE(matches("typedef int X;", typedefDecl(hasType(asString("int");
   EXPECT_TRUE(matches("typedef const int T;",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4400,6 +4400,13 @@
 return;
   }
 
+  DeclarationMatcher ClassHasAnyDirectBase =
+  cxxRecordDecl(hasDirectBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : public virtual X {};",
+  ClassHasAnyDirectBase));
+
   EXPECT_TRUE(matches(
   R"cc(
 class Base {};
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -179,6 +179,7 @@
   REGISTER_MATCHER(cxxConstructExpr);
   REGISTER_MATCHER(cxxConstructorDecl);
   REGISTER_MATCHER(cxxConversionDecl);
+  REGISTER_MATCHER(cxxBaseSpecifier);
   REGISTER_MATCHER(cxxCtorInitializer);
   REGISTER_MATCHER(cxxDeductionGuideDecl);
   REGISTER_MATCHER(cxxDefaultArgExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -756,6 +756,7 @@
 const internal::VariadicDynCastAllOfMatcher parmVarDecl;
 const internal::VariadicDynCastAllOfMatcher
 accessSpecDecl;
+const internal::VariadicAllOfMatcher cxxBaseSpecifier;
 const internal::VariadicAllOfMatcher cxxCtorInitializer;
 const internal::VariadicAllOfMatcher templateArgument;
 const internal::VariadicAllOfMatcher templateArgumentLoc;
Index: 

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 331945.
nick retitled this revision from "[clang][AST] Add `CXXBaseSpecifier` matcher 
support" to "[ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)".
nick edited the summary of this revision.
nick added a reviewer: aaron.ballman.
nick added a comment.

Rebased. Removed things reimplemented and landed in D81552 
 + D79063 .


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

https://reviews.llvm.org/D69218

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -297,6 +297,17 @@
   EXPECT_TRUE(matches("int b[7];", M));
 }
 
+TEST_F(RegistryTest, CXXBaseSpecifier) {
+  // TODO: rewrite with top-level cxxBaseSpecifier matcher when available
+  DeclarationMatcher ClassHasAnyDirectBase =
+  constructMatcher("cxxRecordDecl",
+   constructMatcher("hasDirectBase",
+constructMatcher("cxxBaseSpecifier")))
+  .getTypedMatcher();
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+}
+
 TEST_F(RegistryTest, CXXCtorInitializer) {
   Matcher CtorDecl = constructMatcher(
   "cxxConstructorDecl",
Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -320,6 +320,15 @@
 varDecl(hasType(pointsTo(ClassX);
 }
 
+TEST(HasType, TakesQualTypeMatcherAndMatchesCXXBaseSpecifier) {
+  TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, TakesDeclMatcherAndMatchesExpr) {
   DeclarationMatcher ClassX = recordDecl(hasName("X"));
   EXPECT_TRUE(
@@ -337,6 +346,15 @@
 notMatches("class X {}; void y() { X *x; }", varDecl(hasType(ClassX;
 }
 
+TEST(HasType, TakesDeclMatcherAndMatchesCXXBaseSpecifier) {
+  DeclarationMatcher ClassX = recordDecl(hasName("X"));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, MatchesTypedefDecl) {
   EXPECT_TRUE(matches("typedef int X;", typedefDecl(hasType(asString("int");
   EXPECT_TRUE(matches("typedef const int T;",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1278,6 +1278,35 @@
   ZIsDirectlyDerivedFromX));
 }
 
+TEST_P(ASTMatchersTest, ClassHasBase) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  DeclarationMatcher ClassHasAnyDirectBase =
+  cxxRecordDecl(hasDirectBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : public virtual X {};",
+  ClassHasAnyDirectBase));
+
+  TypeMatcher ClassX = asString("class X");
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : X, Y {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : Y, X {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class W {}; class Y {}; class Z : W, Y {};",
+ ClassHasBaseClassX));
+  DeclarationMatcher ClassZHasBaseClassX =
+  cxxRecordDecl(hasName("Z"), hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {}; class Z : X {};",
+  

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Felix Berger via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:52
 
 AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector,
   NameList) {

hokein wrote:
> worth some comments on this.
Done in its new location.



Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:55
   return llvm::any_of(NameList, [](const std::string ) {
-  return llvm::Regex(Name).match(Node.getName());
-});
+if (Name.find("::") != std::string::npos) {
+  return llvm::Regex(Name).match(Node.getQualifiedNameAsString());

hokein wrote:
> hokein wrote:
> > If we pass the name as a `llvm::StringRef`, we can use `if 
> > (Name.contains("::"))` which seems a bit clearer to me.
> there is a tricky case where the `::` is a prefix of the Name, e.g.  a global 
> symbol x, the Name is `::x`, and `getQualifiedNameAsString` of symbol x is 
> `x` (without the `::` prefix), then the matcher will not find a match, but we 
> expect this is a match right?
> 
I agree that's a tricky case.

We could prepend "::" to getQualifiedNameAsString() and always match against 
it. Would this work?

Or we could do this only for regex names that have a leading "::".  This would 
avoid extra string operations, and users could still match using "^foo::Bar$" 
to match ::foo::Bar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98738

___
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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

To whom it may concern, see D98971 . I'm 
(unwisely?) trying a surgical fix rather than trying to revert the month-old 
thing. (Let's please discuss anything D98971 
-specific over there.)


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] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone created this revision.
Quuxplusone added reviewers: rsmith, nullptr.cpp, aaronpuchert, mizvekov, 
erik.pilkington, sberg, davidstone, david_stone.
Quuxplusone added a project: clang.
Quuxplusone requested review of this revision.
Herald added a subscriber: cfe-commits.

Review D88220  turns out to have some pretty 
severe bugs, but I *think* this patch fixes them.

Paper P1825  is supposed to enable implicit 
move from "non-volatile objects
and rvalue references to non-volatile object types." Instead, what was committed
seems to have enabled implicit move from "non-volatile things of all kinds,
except that if they're rvalue references then they must also refer to 
non-volatile
things." In other words, D88220  accidentally 
enabled implicit move from
lvalue object references (super yikes!) and also from non-object references
(such as references to functions).

These two cases are now fixed and regression-tested.

(For better and worse, D88220  was present in 
trunk for more than a month before anyone noticed the (very major!) bug — my 
thanks to @sberg for reporting it! — so we may be able to take our time fixing 
it, but also it may be another month before someone spots the //next// bug. I 
have no strong opinion on the process here.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98971

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp

Index: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
===
--- clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
+++ clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
@@ -292,3 +292,108 @@
   return b; // cxx20-error {{calling a private constructor of class 'test_ctor_param_rvalue_ref::B2'}}
 }
 } // namespace test_ctor_param_rvalue_ref
+
+namespace test_lvalue_ref_is_not_moved_from {
+
+struct Target {};
+  // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable}}
+  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable}}
+  // cxx11_14_17-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+  // cxx11_14_17-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+struct CopyOnly {
+  CopyOnly(CopyOnly&&) = delete; // cxx20-note {{has been explicitly marked deleted here}}
+  CopyOnly(CopyOnly&);
+  operator Target() && = delete; // cxx20-note {{has been explicitly marked deleted here}}
+  operator Target() &;
+};
+
+struct MoveOnly {
+  MoveOnly(MoveOnly&&); // expected-note {{copy constructor is implicitly deleted because}}
+// cxx11_14_17-note@-1 {{copy constructor is implicitly deleted because}}
+  operator Target() &&; // expected-note {{candidate function not viable}}
+// cxx11_14_17-note@-1 {{candidate function not viable}}
+};
+
+extern CopyOnly copyonly;
+extern MoveOnly moveonly;
+
+CopyOnly t1() {
+CopyOnly& r = copyonly;
+return r;
+}
+
+CopyOnly t2() {
+CopyOnly&& r = static_cast(copyonly);
+return r; // cxx20-error {{call to deleted constructor}}
+}
+
+MoveOnly t3() {
+MoveOnly& r = moveonly;
+return r; // expected-error {{call to implicitly-deleted copy constructor}}
+}
+
+MoveOnly t4() {
+MoveOnly&& r = static_cast(moveonly);
+return r; // cxx11_14_17-error {{call to implicitly-deleted copy constructor}}
+}
+
+Target t5() {
+CopyOnly& r = copyonly;
+return r;
+}
+
+Target t6() {
+CopyOnly&& r = static_cast(copyonly);
+return r; // cxx20-error {{invokes a deleted function}}
+}
+
+Target t7() {
+MoveOnly& r = moveonly;
+return r; // expected-error {{no viable conversion}}
+}
+
+Target t8() {
+MoveOnly&& r = static_cast(moveonly);
+return r; // cxx11_14_17-error {{no viable conversion}}
+}
+
+} // namespace test_lvalue_ref_is_not_moved_from
+
+namespace test_rvalue_ref_to_nonobject {
+
+struct CopyOnly {};
+struct MoveOnly {};
+
+struct Target {
+Target(CopyOnly (&)());
+Target(CopyOnly (&&)()) = delete;
+Target(MoveOnly (&)()) = delete; // expected-note {{has been explicitly marked deleted here}}
+  // expected-note@-1 {{has been explicitly marked deleted here}}
+Target(MoveOnly (&&)());
+};
+
+CopyOnly make_copyonly();
+MoveOnly make_moveonly();
+
+Target t1() {
+CopyOnly ()() = make_copyonly;
+return r;
+}
+
+Target t2() {
+CopyOnly (&)() = static_cast(make_copyonly);
+return r; // OK in all modes; not subject to implicit move
+}
+
+Target t3() {
+MoveOnly ()() = make_moveonly;
+return r; // expected-error {{invokes a deleted function}}
+}
+
+Target t4() {
+MoveOnly (&)() = static_cast(make_moveonly);
+return r; // expected-error {{invokes a deleted function}}
+}
+
+} // namespace test_rvalue_ref_to_nonobject
Index: 

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Felix Berger via Phabricator via cfe-commits]

flx updated this revision to Diff 331941.
flx added a comment.

Applied changes suggested by ymandel, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98738

Files:
  clang-tools-extra/clang-tidy/utils/Matchers.h
  clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp
@@ -1,5 +1,5 @@
 // RUN: %check_clang_tidy %s performance-for-range-copy %t -- \
-// RUN: -config="{CheckOptions: [{key: performance-for-range-copy.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" \
+// RUN: -config="{CheckOptions: [{key: performance-for-range-copy.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$;fully::Qualified'}]}" \
 // RUN: -- -fno-delayed-template-parsing
 
 template 
@@ -63,6 +63,14 @@
 
 typedef SomeComplexTemplate NotTooComplexRef;
 
+namespace fully {
+
+struct Qualified {
+  ~Qualified();
+};
+
+} // namespace fully
+
 void negativeSmartPointer() {
   for (auto P : View>()) {
 auto P2 = P;
@@ -124,3 +132,13 @@
 auto R2 = R;
   }
 }
+
+void negativeFullyQualified() {
+  for (auto Q : View>()) {
+auto Q2 = Q;
+  }
+  using fully::Qualified;
+  for (auto Q : View>()) {
+auto Q2 = Q;
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
@@ -66,4 +66,7 @@
 
A semicolon-separated list of names of types allowed to be passed by value.
Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches every type
-   with suffix `Ref`, `ref`, `Reference` and `reference`. The default is empty.
+   with suffix `Ref`, `ref`, `Reference` and `reference`. The default is
+   empty. If a name in the list contains the sequence `::` it is matched against
+   the qualified typename (i.e. `namespace::Type`, otherwise it is matched
+   against only the type name (i.e. `Type`).
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
@@ -43,5 +43,7 @@
 
A semicolon-separated list of names of types allowed to be initialized by
copying. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches
-   every type with suffix `Ref`, `ref`, `Reference` and `reference`. The
-   default is empty.
+   every type with suffix `Ref`, `ref`, `Reference` and `reference`. The default
+   is empty. If a name in the list contains the sequence `::` it is matched
+   against the qualified typename (i.e. `namespace::Type`, otherwise it is
+   matched against only the type name (i.e. `Type`).
Index: clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
@@ -31,4 +31,6 @@
A semicolon-separated list of names of types allowed to be copied in each
iteration. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches
every type with suffix `Ref`, `ref`, `Reference` and `reference`. The default
-   is empty.
+   is empty. If a name in the list contains the sequence `::` it is matched
+   against the qualified typename (i.e. `namespace::Type`, otherwise it is
+   matched against only the type name (i.e. `Type`).
Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -49,11 +49,46 @@
   return pointerType(pointee(qualType(isConstQualified(;
 }
 
-AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector,
-  NameList) {
-  return llvm::any_of(NameList, [](const std::string ) {
-  return llvm::Regex(Name).match(Node.getName());
+class MatchesAnyListedNameMatcher
+: public 

[PATCH] D98923: [Driver] Pass -fexperimental-strict-floating-point to cc1 if it is specified

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Isn't OPT_fexperimental_strict_floating_point marked as a CC1Option in 
Options.td. Can the driver even recognize it?

Can you use -Xclang -fexperimental-strict-floating-point for your use case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98923

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

dblaikie wrote:
> hoy wrote:
> > dblaikie wrote:
> > > Does this need to be down here? Or would the code be a well exercised if 
> > > it was up next to the go declaration above?
> > Yes, it needs to be here. Otherwise it will just like the function `bar` 
> > above that doesn't get a uniquefied name. I think moving the definition up 
> > to right after the declaration hides the declaration.
> Not sure I follow - do you mean that if the go declaration and go definition 
> were next to each other, this test would (mechanically speaking) not validate 
> what the patch? Or that it would be less legible, but still mechanically 
> correct?
> 
> I think it would be (assuming it's still mechanically correct) more legible 
> to put the declaration next to the definition - the comment describes why the 
> declaration is significant/why the definition is weird, and seeing all that 
> together would be clearer to me than spreading it out/having to look further 
> away to see what's going on.
When the `go` declaration and `go` definition were next to each other, the go 
function won't get a uniqufied name at all. The declaration will be overwritten 
by the definition. Only when the declaration is seen by others, such the 
callsite in `baz`, the declaration makes a difference by having the callsite 
use a uniqufied name.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[PATCH] D98923: [Driver] Pass -fexperimental-strict-floating-point to cc1 if it is specified

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

Test case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98923

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D98278#2637866 , @zero9178 wrote:

> In D98278#2637826 , @mstorsjo wrote:
>
>> Btw, while this change does explain _what_ it does, it doesn't actually say 
>> the exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case 
>> where e.g. some translations produce different error messages?
>
> Now that you mention it, it's indeed not as clear as I thought. But yes, in 
> the case of MSVCs STL, the messages from `std::error_codes` which are used by 
> various LLVM tools produce different strings then using `strerror` (the C 
> function also called by Python) with the same error codes (Specifically, it 
> has different casing).

Or turning the question another way around: We have a couple different bots 
that build and run the tests, successfully, with MSVC configurations. Are there 
tests that failed for you in your configuration, that succeed in the setup of 
the bots? Or are there other tests that aren't run as part of bots that you're 
fixing? It's all still very vague to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D98278#2637866 , @zero9178 wrote:

> In D98278#2637826 , @mstorsjo wrote:
>
>> Btw, while this change does explain _what_ it does, it doesn't actually say 
>> the exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case 
>> where e.g. some translations produce different error messages?
>
> Now that you mention it, it's indeed not as clear as I thought. But yes, in 
> the case of MSVCs STL, the messages from `std::error_codes` which are used by 
> various LLVM tools produce different strings then using `strerror` (the C 
> function also called by Python) with the same error codes (Specifically, it 
> has different casing).

Ok, but would e.g. a case insensitive comparison have worked instead of this?

And didn't the python script have hardcoded strings, specifically for the MSVC 
case? Why weren't they written with the right casing for the case that they're 
supposed to match? I.e. was it an issue with the existing hardcoded strings, or 
did they work in one case but not another one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98278

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

[PATCH] D98948: [analyzer][solver] Fix infeasible constraints (PR49642)

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

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

Aa, get it. Looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98948

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

hoy wrote:
> dblaikie wrote:
> > Does this need to be down here? Or would the code be a well exercised if it 
> > was up next to the go declaration above?
> Yes, it needs to be here. Otherwise it will just like the function `bar` 
> above that doesn't get a uniquefied name. I think moving the definition up to 
> right after the declaration hides the declaration.
Not sure I follow - do you mean that if the go declaration and go definition 
were next to each other, this test would (mechanically speaking) not validate 
what the patch? Or that it would be less legible, but still mechanically 
correct?

I think it would be (assuming it's still mechanically correct) more legible to 
put the declaration next to the definition - the comment describes why the 
declaration is significant/why the definition is weird, and seeing all that 
together would be clearer to me than spreading it out/having to look further 
away to see what's going on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[PATCH] D98902: [Clang][OpenMP][NVPTX] Fixed failure in openmp-offload-gpu.c if the system has CUDA

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D98902#2636308 , @jdoerfert wrote:

> @tra, so you think we should not do this? The user will see a link error late 
> I assume, might be better.

If I compile a `__device__ float foo(float f) { return sin(f); }` and it does 
compile to working GPU code, I If I compile the same code with `-S`, I would 
assume that produced PTX is still compileable with ptxas. After all, it was 
when the source was compiled with `-c`. That will no longer be the case if you 
disable linking with libdevice.

If the user wants to disable linking with the libdevice, there's already 
`-nogpulib` for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98902

___
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

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:298
+  driverPath.append("/../include/flang/");
+  return driverPath.str().str();
+}

Given this [[ 
https://github.com/llvm/llvm-project/blob/987ee6e3cc1fb672b3ed201e72a5281c2ec88c99/llvm/include/llvm/ADT/SmallString.h#L271-L273
 | conversion operator ]], wouldn't `return std::string(driverPath);` be more 
efficient? 


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

https://reviews.llvm.org/D97080

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

[PATCH] D97680: [OpenMP] Simplify GPU memory globalization

[Jose Manuel Monsalve Diaz via Phabricator via cfe-commits]

josemonsalve2 added inline comments.



Comment at: clang/test/OpenMP/nvptx_parallel_codegen.cpp:4
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - -disable-llvm-optzns | FileCheck 
%s --check-prefix CHECK --check-prefix CHECK-64 --check-prefix SEQ
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - -disable-llvm-optzns 
-fopenmp-cuda-parallel-target-regions | FileCheck %s --check-prefix CHECK 
--check-prefix CHECK-64 --check-prefix PAR
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple i386-unknown-unknown 
-fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc

Is this flag `-fopenmp-cuda-parallel-target-regions` useful after this change? 
I know it was used to determine something in globalization, and I believe this 
was removed. But is it used for anything else somewhere else or could it be 
removed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97680

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

[PATCH] D98867: [HIP] Fix ROCm detection

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:267-268
+// rocm-{major}.{minor}.{subMinor}[-{build}]
+auto Loc = StringRef(VerStr).rfind('_');
+if (Loc != StringRef::npos)
+  VerStr[Loc] = '.';

You don't need `StringRef` here. `VerStr.rfind()` will do.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:269
+if (Loc != StringRef::npos)
+  VerStr[Loc] = '.';
+V.tryParse(VerStr);

This only deals with a single `.` in the version. It's likely that we will have 
to deal with a patch release at some point, so you may want to replace all 
instances.



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

https://reviews.llvm.org/D98867

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

  enum {
  A,
  B,
  C,
  } ShortEnum1,
  ShortEnum2;

I've seen this before maybe with regard to something else, but can't quite 
recall. (maybe a bug in the bug tracker)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

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

zero9178 added a comment.

In D98278#2637826 , @mstorsjo wrote:

> Btw, while this change does explain _what_ it does, it doesn't actually say 
> the exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case 
> where e.g. some translations produce different error messages?

Now that you mention it, it's indeed not as clear as I thought. But yes, in the 
case of MSVCs STL, the messages from `std::error_codes` which are used by 
various LLVM tools produce different strings then using `strerror` (the C 
function also called by Python) with the same error codes (Specifically, it has 
different casing).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98278

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

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2132
+**EmptyLineAfterAccessModifier** (``EmptyLineAfterAccessModifierStyle``)
+  Defines in which cases to put empty line after access modifiers.
+





Comment at: clang/docs/ClangFormatStyleOptions.rst:2159
+Always add empty line after access modifiers if there are none.
+MaxEmptyLinesToKeep is applied also.
+

if MaxEmptyLineToKeep is 0 then surely ELAAMS_Always should win, as this is the 
lesser i.e. if people never uses extra lines except after access modifiers this 
is what they would set? did I missunderstand?

otherwise doesn't MaxEmptyLinesToKeep just nullify the whole option? meaning 
its useless for anyone using it?

I agree in the ELAAMS_Leave  case that MaxEmptyLinesToKeep  should win.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

We're seeing a crash in the optimizer after this patch, with the following 
logged in assert builds: `assert.h assertion failed at 
llvm/include/llvm/Support/Casting.h:104 in static bool 
llvm::isa_impl_cl::doit(const From 
*) [To = llvm::InvokeInst, From = const llvm::Instruction *]: Val && "isa<> 
used on a null pointer"`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98816

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

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

could you please mark your comments done when they are done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added inline comments.
This revision now requires changes to proceed.



Comment at: clang/unittests/Format/FormatTest.cpp:9392
+ "};\n";
+  EXPECT_EQ(NL_B_3_A_1_I_3, format(NL_B_3_A_3_I_3, Style));
+

I can't read this, I can't tell if you've fat fingered a test, I feel these are 
better done systematically one by one with the text in the verifyFormat("...") 
it makes it easier when they fail to understand what is going on.

Also its ok to have more than one TEST_F if you want to handle the different 
states in small batches

I think you undetersand what NL_B_3_A_0_I_0 means and how it differs form 
NL_B_0_A_3_I_0  but others won't


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Btw, while this change does explain _what_ it does, it doesn't actually say the 
exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case where 
e.g. some translations produce different error messages?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98278

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

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:9212
+  StyleWithLine.EmptyLinesAfterAccessModifier = 1u;
+  EXPECT_EQ(test2NL, format(test0NL, StyleWithLine));
+  EXPECT_EQ(test2NL, format(test1NL, StyleWithLine));

Max_S wrote:
> HazardyKnusperkeks wrote:
> > Max_S wrote:
> > > MyDeveloperDay wrote:
> > > > yeah I'm not a fan of this like this... sorry... just write the test 
> > > > out in long form, when it goes wrong I don't have to be a compiler to 
> > > > understand what is going wrong I can just see it.
> > > I can change this, but the current output of the tests is (I forced the 
> > > error):
> > > ```
> > > //llvm-project/clang/unittests/Format/FormatTest.cpp:72: Failure
> > >   Expected: Expected.str()
> > >   Which is: "class Foo {\nprivate:\n\n  int i;\n};"
> > > To be equal to: format(Expected, Style)
> > >   Which is: "class Foo {\nprivate:\n  int i;\n};"
> > > With diff:
> > > @@ -1,5 @@
> > >  class Foo {
> > >  private:
> > > -
> > >int i;
> > >  };
> > > ```
> > > 
> > > Which is actually human readable in this case. Shall I still change it?
> > I'm a fan of it. :)
> > Especially with the demonstration that the string is still expanded.
> I changed all to EXPECT_EQ since the failer output there is better. It shows 
> the variables names and not the arbitrary code in verfyFormat. Hope this is 
> ok.
> ```
> /home/msagebaum/Kaiserslautern/Programms/temp/llvm-project/clang/unittests/Format/FormatTest.cpp:9202:
>  Failure
>   Expected: test2NL
>   Which is: "class Foo {\nprivate:\n\n  int i;\n};"
> To be equal to: format(test1NL)
>   Which is: "class Foo {\nprivate:\n  int i;\n};"
> With diff:
> @@ -1,5 @@
>  class Foo {
>  private:
> -
>int i;
>  };
> 
> 
why would we break with convention?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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

[PATCH] D98868: [Driver] Add -print-runtime-dir

[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 rGaafc3f7be804: [Driver] Add -print-runtime-dir (authored by 
zero9178).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98868

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/resource_dir/lib/windows/clang_rt.builtins-x86_64.lib
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-pc-windows-msvc/clang_rt.builtins.lib
  clang/test/Driver/immediate-options.c


Index: clang/test/Driver/immediate-options.c
===
--- clang/test/Driver/immediate-options.c
+++ clang/test/Driver/immediate-options.c
@@ -17,3 +17,15 @@
 // Allow unspecified output because the value of CLANG_RESOURCE_DIR is unknown.
 // RUN: %clang -print-resource-dir | FileCheck %s 
-check-prefix=PRINT-RESOURCE-DIR
 // PRINT-RESOURCE-DIR: {{.+}}
+
+// Default resource-dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR %s
+// PRINT-RUNTIME-DIR: lib{{/|\\}}windows
+
+// Per target dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR-PER-TARGET %s
+// PRINT-RUNTIME-DIR-PER-TARGET: lib{{/|\\}}x86_64-pc-windows-msvc
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1824,6 +1824,15 @@
 return false;
   }
 
+  if (C.getArgs().hasArg(options::OPT_print_runtime_dir)) {
+if (auto RuntimePath = TC.getRuntimePath()) {
+  llvm::outs() << *RuntimePath << '\n';
+  return false;
+}
+llvm::outs() << TC.getCompilerRTPath() << '\n';
+return false;
+  }
+
   // FIXME: The following handlers should use a callback mechanism, we don't
   // know what the client would like to do.
   if (Arg *A = C.getArgs().getLastArg(options::OPT_print_file_name_EQ)) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3566,6 +3566,8 @@
   HelpText<"Print the registered targets">;
 def print_rocm_search_dirs : Flag<["-", "--"], "print-rocm-search-dirs">,
   HelpText<"Print the paths used for finding ROCm installation">;
+def print_runtime_dir : Flag<["-", "--"], "print-runtime-dir">,
+  HelpText<"Print the directory pathname containing clangs runtime libraries">;
 def private__bundle : Flag<["-"], "private_bundle">;
 def pthreads : Flag<["-"], "pthreads">;
 defm pthread : BoolOption<"", "pthread",


Index: clang/test/Driver/immediate-options.c
===
--- clang/test/Driver/immediate-options.c
+++ clang/test/Driver/immediate-options.c
@@ -17,3 +17,15 @@
 // Allow unspecified output because the value of CLANG_RESOURCE_DIR is unknown.
 // RUN: %clang -print-resource-dir | FileCheck %s -check-prefix=PRINT-RESOURCE-DIR
 // PRINT-RESOURCE-DIR: {{.+}}
+
+// Default resource-dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR %s
+// PRINT-RUNTIME-DIR: lib{{/|\\}}windows
+
+// Per target dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR-PER-TARGET %s
+// PRINT-RUNTIME-DIR-PER-TARGET: lib{{/|\\}}x86_64-pc-windows-msvc
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1824,6 +1824,15 @@
 return false;
   }
 
+  if (C.getArgs().hasArg(options::OPT_print_runtime_dir)) {
+if (auto RuntimePath = TC.getRuntimePath()) {
+  llvm::outs() << *RuntimePath << '\n';
+  return false;
+}
+llvm::outs() << TC.getCompilerRTPath() << '\n';
+return false;
+  }
+
   // FIXME: The following handlers should use a callback mechanism, we don't
   // know what the client would like to do.
   if (Arg *A = C.getArgs().getLastArg(options::OPT_print_file_name_EQ)) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3566,6 +3566,8 @@
   HelpText<"Print the registered targets">;
 def print_rocm_search_dirs : 

[clang] aafc3f7 - [Driver] Add -print-runtime-dir

[Markus Böck via cfe-commits]

Author: Markus Böck
Date: 2021-03-19T17:48:03+01:00
New Revision: aafc3f7be804d117a632365489a18c3e484a3931

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

LOG: [Driver] Add -print-runtime-dir

This patch adds a new command line option to clang which outputs the directory 
containing clangs runtime libraries to stdout.

The primary use case for this command line flag is for build systems using 
clang-cl. Build systems when using clang-cl invoke the linker, that is either 
link or lld-link in this case, directly instead of invoking the compiler for 
the linking process as is common with the other drivers. This leads to issues 
when runtime libraries of clang, such as sanitizers or profiling, have to be 
linked in as the compiler cannot communicate the link directory to the linker.

Using this flag, build systems would be capable of getting the directory 
containing all of clang's runtime libraries and add it to the linker path.

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

Added: 

clang/test/Driver/Inputs/resource_dir/lib/windows/clang_rt.builtins-x86_64.lib

clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-pc-windows-msvc/clang_rt.builtins.lib

Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/Driver.cpp
clang/test/Driver/immediate-options.c

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index a9b43a8fe620..b7efb7469a23 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3566,6 +3566,8 @@ def print_targets : Flag<["-", "--"], "print-targets">,
   HelpText<"Print the registered targets">;
 def print_rocm_search_dirs : Flag<["-", "--"], "print-rocm-search-dirs">,
   HelpText<"Print the paths used for finding ROCm installation">;
+def print_runtime_dir : Flag<["-", "--"], "print-runtime-dir">,
+  HelpText<"Print the directory pathname containing clangs runtime libraries">;
 def private__bundle : Flag<["-"], "private_bundle">;
 def pthreads : Flag<["-"], "pthreads">;
 defm pthread : BoolOption<"", "pthread",

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index dbd365e7c9bc..e70263e6a295 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1824,6 +1824,15 @@ bool Driver::HandleImmediateArgs(const Compilation ) {
 return false;
   }
 
+  if (C.getArgs().hasArg(options::OPT_print_runtime_dir)) {
+if (auto RuntimePath = TC.getRuntimePath()) {
+  llvm::outs() << *RuntimePath << '\n';
+  return false;
+}
+llvm::outs() << TC.getCompilerRTPath() << '\n';
+return false;
+  }
+
   // FIXME: The following handlers should use a callback mechanism, we don't
   // know what the client would like to do.
   if (Arg *A = C.getArgs().getLastArg(options::OPT_print_file_name_EQ)) {

diff  --git 
a/clang/test/Driver/Inputs/resource_dir/lib/windows/clang_rt.builtins-x86_64.lib
 
b/clang/test/Driver/Inputs/resource_dir/lib/windows/clang_rt.builtins-x86_64.lib
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-pc-windows-msvc/clang_rt.builtins.lib
 
b/clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-pc-windows-msvc/clang_rt.builtins.lib
new file mode 100644
index ..e69de29bb2d1

diff  --git a/clang/test/Driver/immediate-options.c 
b/clang/test/Driver/immediate-options.c
index 723a6fa302f8..c398e0d41c6e 100644
--- a/clang/test/Driver/immediate-options.c
+++ b/clang/test/Driver/immediate-options.c
@@ -17,3 +17,15 @@
 // Allow unspecified output because the value of CLANG_RESOURCE_DIR is unknown.
 // RUN: %clang -print-resource-dir | FileCheck %s 
-check-prefix=PRINT-RESOURCE-DIR
 // PRINT-RESOURCE-DIR: {{.+}}
+
+// Default resource-dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR %s
+// PRINT-RUNTIME-DIR: lib{{/|\\}}windows
+
+// Per target dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR-PER-TARGET %s
+// PRINT-RUNTIME-DIR-PER-TARGET: lib{{/|\\}}x86_64-pc-windows-msvc



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

[PATCH] D97119: [flang][driver] Add options for -std=f2018

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/test/Driver/std2018.f90:1
+! REQUIRES: new-flang-driver
+! Ensure argument -std=f2018 works as expected.

Would it be possible to share this test with `f18`?



Comment at: flang/test/Driver/std2018_wrong.f90:14-22
+subroutine foo2()
+do 01 m=1,2
+  select case (m)
+  case default
+print*, "default", m
+  case (1)
+print*, "start"

No input is needed for this test, is it? IIUC, this bit can be deleted to make 
the test leaner.


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

https://reviews.llvm.org/D97119

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

[PATCH] D98951: [clang][ASTImporter] Add import API for 'const Type *' (NFC).

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik accepted this revision.
shafik added a comment.

Can we add a test for the `getCapturedVLAType` case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98951

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

[PATCH] D98856: Always emit error for wrong interfaces to scalable vectors, unless cmdline flag is passed.

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5052-5055
+  if (isa(JA)) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-treat-scalable-fixed-error-as-warning");
+  }

Are there any concerns related to LTO here? Could we live with LTO triggering 
errors for the invalid uses?  Part of me thinks this is reasonable given the 
clang exception is more about ensuring we can continue active development until 
we're ready to press the "it's supported" switch.



Comment at: llvm/include/llvm/Support/TypeSize.h:31-35
+namespace TypeSizeClOpt {
+/// The ScalableErrorAsWarning is a temporary measure to suppress errors from
+/// using the wrong interface.
+extern cl::opt ScalableErrorAsWarning;
+} // namespace TypeSizeClOpt

Why is this need here? Can it just be externs where it's accessed?



Comment at: llvm/lib/CodeGen/ValueTypes.cpp:28
+  if (isScalableVector()) {
+if (llvm::TypeSizeClOpt::ScalableErrorAsWarning)
+  WithColor::warning()

I guess related to my comment for TypeSize.h but I'm wondering if it's better 
to move all error reporting into TypeSize.cpp. For example:
```
if (isScalableVector())
  reportInvalidSizeRequest("EVT::getVectorNumElements()", 
"EVT::getVectorElementCount()")

reportInvalidSizeRequest(string bad_func, string good_fun) {
#ifndef STRICT_FIXED_SIZE_VECTORS
  if (ScalableErrorAsWarning)
warning(don't use badfunc, use good_fun instead)
  else
#endif
Error()
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98856

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

[PATCH] D98856: Always emit error for wrong interfaces to scalable vectors, unless cmdline flag is passed.

[Christopher Tetreault via Phabricator via cfe-commits]

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

lgtm




Comment at: llvm/lib/CodeGen/ValueTypes.cpp:17
 
+unsigned EVT::getVectorNumElements() const {
+  auto Error = []() {

Out of curiosity, what is the eventual plan for this function? Does it go away, 
or will it just assert if this is a scalable vector?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98856

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

[PATCH] D98868: [Driver] Add -print-runtime-dir

[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

I'm somewhat surprised this doesn't exist already, but I looked at the -print-* 
options and don't see anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98868

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

[PATCH] D98959: [OpenCL] Add as_size/ptrdiff/intptr/uintptr_t operators

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added a reviewer: Anastasia.
svenvh added a project: clang.
Herald added subscribers: ldrumm, yaxunl.
svenvh requested review of this revision.
Herald added a subscriber: cfe-commits.

size_t and friends are built-in scalar data types and s6.4.4.2 of the
OpenCL C Specification says the `as_type()` operator must be available
for these data types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98959

Files:
  clang/lib/Headers/opencl-c-base.h
  clang/test/SemaOpenCL/as_type.cl


Index: clang/test/SemaOpenCL/as_type.cl
===
--- clang/test/SemaOpenCL/as_type.cl
+++ clang/test/SemaOpenCL/as_type.cl
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown 
-finclude-default-header -o - -verify -fsyntax-only
+// RUN: %clang_cc1 %s -emit-llvm -DBITS=32 -triple spir-unknown-unknown 
-finclude-default-header -fdeclare-opencl-builtins -o - -verify -fsyntax-only
+// RUN: %clang_cc1 %s -emit-llvm -DBITS=64 -triple spir64-unknown-unknown 
-finclude-default-header -fdeclare-opencl-builtins -o - -verify -fsyntax-only
 
 char3 f1(char16 x) {
   return  __builtin_astype(x, char3); // expected-error{{invalid 
reinterpretation: sizes of 'char3' (vector of 3 'char' values) and '__private 
char16' (vector of 16 'char' values) must match}}
@@ -12,3 +13,15 @@
 char src = 1;
 int dst = as_int(src); // expected-error{{invalid reinterpretation: sizes 
of 'int' and '__private char' must match}}
 }
+
+void target_dependent(int i, long l) {
+  size_t size1 = as_size_t(i);
+#if BITS == 64
+  // expected-error@-2{{sizes of 'size_t' (aka 'unsigned long') and '__private 
int' must match}}
+#endif
+
+  size_t size2 = as_size_t(l);
+#if BITS == 32
+  // expected-error@-2{{sizes of 'size_t' (aka 'unsigned int') and '__private 
long' must match}}
+#endif
+}
Index: clang/lib/Headers/opencl-c-base.h
===
--- clang/lib/Headers/opencl-c-base.h
+++ clang/lib/Headers/opencl-c-base.h
@@ -545,6 +545,11 @@
 #define as_half16(x) __builtin_astype((x), half16)
 #endif // cl_khr_fp16
 
+#define as_size_t(x) __builtin_astype((x), size_t)
+#define as_ptrdiff_t(x) __builtin_astype((x), ptrdiff_t)
+#define as_intptr_t(x) __builtin_astype((x), intptr_t)
+#define as_uintptr_t(x) __builtin_astype((x), uintptr_t)
+
 // OpenCL v1.1 s6.9, v1.2/2.0 s6.10 - Function qualifiers
 
 #define __kernel_exec(X, typen) __kernel \


Index: clang/test/SemaOpenCL/as_type.cl
===
--- clang/test/SemaOpenCL/as_type.cl
+++ clang/test/SemaOpenCL/as_type.cl
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -finclude-default-header -o - -verify -fsyntax-only
+// RUN: %clang_cc1 %s -emit-llvm -DBITS=32 -triple spir-unknown-unknown -finclude-default-header -fdeclare-opencl-builtins -o - -verify -fsyntax-only
+// RUN: %clang_cc1 %s -emit-llvm -DBITS=64 -triple spir64-unknown-unknown -finclude-default-header -fdeclare-opencl-builtins -o - -verify -fsyntax-only
 
 char3 f1(char16 x) {
   return  __builtin_astype(x, char3); // expected-error{{invalid reinterpretation: sizes of 'char3' (vector of 3 'char' values) and '__private char16' (vector of 16 'char' values) must match}}
@@ -12,3 +13,15 @@
 char src = 1;
 int dst = as_int(src); // expected-error{{invalid reinterpretation: sizes of 'int' and '__private char' must match}}
 }
+
+void target_dependent(int i, long l) {
+  size_t size1 = as_size_t(i);
+#if BITS == 64
+  // expected-error@-2{{sizes of 'size_t' (aka 'unsigned long') and '__private int' must match}}
+#endif
+
+  size_t size2 = as_size_t(l);
+#if BITS == 32
+  // expected-error@-2{{sizes of 'size_t' (aka 'unsigned int') and '__private long' must match}}
+#endif
+}
Index: clang/lib/Headers/opencl-c-base.h
===
--- clang/lib/Headers/opencl-c-base.h
+++ clang/lib/Headers/opencl-c-base.h
@@ -545,6 +545,11 @@
 #define as_half16(x) __builtin_astype((x), half16)
 #endif // cl_khr_fp16
 
+#define as_size_t(x) __builtin_astype((x), size_t)
+#define as_ptrdiff_t(x) __builtin_astype((x), ptrdiff_t)
+#define as_intptr_t(x) __builtin_astype((x), intptr_t)
+#define as_uintptr_t(x) __builtin_astype((x), uintptr_t)
+
 // OpenCL v1.1 s6.9, v1.2/2.0 s6.10 - Function qualifiers
 
 #define __kernel_exec(X, typen) __kernel \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98951: [clang][ASTImporter] Add import API for 'const Type *' (NFC).

[Raphael Isemann via Phabricator via cfe-commits]

teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: whisperity.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98951

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

[PATCH] D98892: [HWASan] Mention x86_64 aliasing mode in design doc.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 331895.
morehouse marked an inline comment as done.
morehouse added a comment.

- Expand on lack of 32 bit support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98892

Files:
  clang/docs/HardwareAssistedAddressSanitizerDesign.rst


Index: clang/docs/HardwareAssistedAddressSanitizerDesign.rst
===
--- clang/docs/HardwareAssistedAddressSanitizerDesign.rst
+++ clang/docs/HardwareAssistedAddressSanitizerDesign.rst
@@ -19,13 +19,17 @@
 sources of AddressSanitizer's memory overhead.
 See the `AddressSanitizer paper`_ for details.
 
-AArch64 has the `Address Tagging`_ (or top-byte-ignore, TBI), a hardware 
feature that allows
-software to use 8 most significant bits of a 64-bit pointer as
+AArch64 has `Address Tagging`_ (or top-byte-ignore, TBI), a hardware feature 
that allows
+software to use the 8 most significant bits of a 64-bit pointer as
 a tag. HWASAN uses `Address Tagging`_
 to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
 but with smaller memory overhead and slightly different (mostly better)
 accuracy guarantees.
 
+Intel's `Linear Address Masking`_ (LAM) also provides address tagging for
+x86_64, though it is not widely available in hardware yet.  For x86_64, HWASAN
+has a limited implementation using page aliasing instead.
+
 Algorithm
 =
 * Every heap/stack/global memory object is forcibly aligned by `TG` bytes
@@ -266,7 +270,15 @@
 will have limited deployability since not all of the code is
 typically instrumented.
 
-The HWASAN's approach is not applicable to 32-bit architectures.
+On x86_64, HWASAN utilizes page aliasing to place tags in userspace address
+bits.  Currently only heap tagging is supported.  The page aliases rely on
+shared memory, which will cause heap memory to be shared between processes if
+the application calls ``fork()``.  Therefore x86_64 is really only safe for
+applications that do not fork.
+
+HWASAN does not currently support 32-bit architectures since they do not
+support `Address Tagging`_ and the address space is too constrained to easily
+implement page aliasing.
 
 
 Related Work
@@ -284,4 +296,4 @@
 .. _SPARC ADI: 
https://lazytyped.blogspot.com/2017/09/getting-started-with-adi.html
 .. _AddressSanitizer paper: 
https://www.usenix.org/system/files/conference/atc12/atc12-final39.pdf
 .. _Address Tagging: 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0024a/ch12s05s01.html
-
+.. _Linear Address Masking: 
https://software.intel.com/content/www/us/en/develop/download/intel-architecture-instruction-set-extensions-programming-reference.html


Index: clang/docs/HardwareAssistedAddressSanitizerDesign.rst
===
--- clang/docs/HardwareAssistedAddressSanitizerDesign.rst
+++ clang/docs/HardwareAssistedAddressSanitizerDesign.rst
@@ -19,13 +19,17 @@
 sources of AddressSanitizer's memory overhead.
 See the `AddressSanitizer paper`_ for details.
 
-AArch64 has the `Address Tagging`_ (or top-byte-ignore, TBI), a hardware feature that allows
-software to use 8 most significant bits of a 64-bit pointer as
+AArch64 has `Address Tagging`_ (or top-byte-ignore, TBI), a hardware feature that allows
+software to use the 8 most significant bits of a 64-bit pointer as
 a tag. HWASAN uses `Address Tagging`_
 to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
 but with smaller memory overhead and slightly different (mostly better)
 accuracy guarantees.
 
+Intel's `Linear Address Masking`_ (LAM) also provides address tagging for
+x86_64, though it is not widely available in hardware yet.  For x86_64, HWASAN
+has a limited implementation using page aliasing instead.
+
 Algorithm
 =
 * Every heap/stack/global memory object is forcibly aligned by `TG` bytes
@@ -266,7 +270,15 @@
 will have limited deployability since not all of the code is
 typically instrumented.
 
-The HWASAN's approach is not applicable to 32-bit architectures.
+On x86_64, HWASAN utilizes page aliasing to place tags in userspace address
+bits.  Currently only heap tagging is supported.  The page aliases rely on
+shared memory, which will cause heap memory to be shared between processes if
+the application calls ``fork()``.  Therefore x86_64 is really only safe for
+applications that do not fork.
+
+HWASAN does not currently support 32-bit architectures since they do not
+support `Address Tagging`_ and the address space is too constrained to easily
+implement page aliasing.
 
 
 Related Work
@@ -284,4 +296,4 @@
 .. _SPARC ADI: https://lazytyped.blogspot.com/2017/09/getting-started-with-adi.html
 .. _AddressSanitizer paper: https://www.usenix.org/system/files/conference/atc12/atc12-final39.pdf
 .. _Address Tagging: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0024a/ch12s05s01.html
-
+.. 

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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

Confirmed: yikes. @nullptr.cpp, I'm going to open a new pull request to revert 
this, to poke the buildbot, just in case reverting it causes new failures. But 
I certainly don't object if someone wants to be more bold than me in reverting 
quickly.
Here's a reduced test case: https://godbolt.org/z/dz43W4

  struct A {
  A(A&&);
  A(const A&);
  };
  
  extern A global;
  
  A f() {
  A& r = global;
  return r;
  }


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] D98867: [HIP] Fix ROCm detection

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 331889.
yaxunl marked an inline comment as done.
yaxunl added a comment.

fix version comparison


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

https://reviews.llvm.org/D98867

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  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
@@ -21,6 +21,20 @@
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,NODEFAULTLIBS %s
 
+// Test environment variable ROCM_PATH.
+// RUN: env ROCM_PATH=%S/Inputs/rocm %clang -### -target x86_64-linux-gnu \
+// RUN:   --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-ENV %s
+
+// Test detecting latest /opt/rocm-{release} directory.
+// RUN: rm -rf %T/opt
+// RUN: mkdir -p %T/opt
+// RUN: cp -r %S/Inputs/rocm %T/opt/rocm-3.9.0-1234
+// RUN: cp -r %S/Inputs/rocm %T/opt/rocm-3.10.0
+// RUN: %clang -### -target x86_64-linux-gnu --sysroot=%T \
+// RUN:   --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-REL %s
+
 // Test ROCm installation built by SPACK by invoke clang at 
%T/rocm-spack/llvm-amdgpu-*
 // directory through a soft link.
 
@@ -60,6 +74,11 @@
 
 // COMMON: "-triple" "amdgcn-amd-amdhsa"
 
+// ROCM-ENV: ROCm installation search path: {{.*}}/Inputs/rocm
+
+// ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm
+// ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm-3.10.0
+
 // SPACK: ROCm installation search path (Spack 4.0.0): [[DIR:.*]]
 // SPACK: ROCm installation search path: [[CLANG:.*]]
 // SPACK: ROCm installation search path: [[CLANG]]/lib/clang/{{[0-9.]+}}
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -186,6 +186,12 @@
 ROCmSearchDirs.emplace_back(RocmPathArg.str());
 DoPrintROCmSearchDirs();
 return ROCmSearchDirs;
+  } else if (const char *RocmPathEnv = ::getenv("ROCM_PATH")) {
+if (!StringRef(RocmPathEnv).empty()) {
+  ROCmSearchDirs.emplace_back(RocmPathEnv);
+  DoPrintROCmSearchDirs();
+  return ROCmSearchDirs;
+}
   }
 
   // Try to find relative to the compiler binary.
@@ -247,6 +253,45 @@
 
   ROCmSearchDirs.emplace_back(D.SysRoot + "/opt/rocm",
   /*StrictChecking=*/true);
+
+  // Find the latest /opt/rocm-{release} directory.
+  std::error_code EC;
+  std::string LatestROCm;
+  llvm::VersionTuple LatestVer;
+  // Get ROCm version from ROCm directory name.
+  auto GetROCmVersion = [](StringRef DirName) {
+llvm::VersionTuple V;
+std::string VerStr = DirName.drop_front(strlen("rocm-")).str();
+// The ROCm directory name follows the format of
+// rocm-{major}.{minor}.{subMinor}[-{build}]
+auto Loc = StringRef(VerStr).rfind('_');
+if (Loc != StringRef::npos)
+  VerStr[Loc] = '.';
+V.tryParse(VerStr);
+return V;
+  };
+  for (llvm::vfs::directory_iterator
+   File = D.getVFS().dir_begin(D.SysRoot + "/opt", EC),
+   FileEnd;
+   File != FileEnd && !EC; File.increment(EC)) {
+llvm::StringRef FileName = llvm::sys::path::filename(File->path());
+if (!FileName.startswith("rocm-"))
+  continue;
+if (LatestROCm.empty()) {
+  LatestROCm = FileName.str();
+  LatestVer = GetROCmVersion(LatestROCm);
+  continue;
+}
+auto Ver = GetROCmVersion(FileName);
+if (LatestVer < Ver) {
+  LatestROCm = FileName.str();
+  LatestVer = Ver;
+}
+  }
+  if (!LatestROCm.empty())
+ROCmSearchDirs.emplace_back(D.SysRoot + "/opt/" + LatestROCm,
+/*StrictChecking=*/true);
+
   DoPrintROCmSearchDirs();
   return ROCmSearchDirs;
 }


Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -21,6 +21,20 @@
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,NODEFAULTLIBS %s
 
+// Test environment variable ROCM_PATH.
+// RUN: env ROCM_PATH=%S/Inputs/rocm %clang -### -target x86_64-linux-gnu \
+// RUN:   --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-ENV %s
+
+// Test detecting latest /opt/rocm-{release} directory.
+// RUN: rm -rf %T/opt
+// RUN: mkdir -p %T/opt
+// RUN: cp -r %S/Inputs/rocm %T/opt/rocm-3.9.0-1234
+// RUN: cp -r %S/Inputs/rocm %T/opt/rocm-3.10.0
+// RUN: %clang -### -target x86_64-linux-gnu --sysroot=%T \
+// RUN:   --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-REL %s
+
 // Test ROCm installation built by SPACK by invoke clang at %T/rocm-spack/llvm-amdgpu-*
 // directory through a soft link.
 
@@ -60,6 +74,11 @@
 
 

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

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:292
+  std::string driverPath = llvm::sys::fs::getMainExecutable(nullptr, nullptr);
+  driverPath = driverPath.substr(0, driverPath.size() - 9);
+  return driverPath.append("/../include/flang/");

awarzynski wrote:
> arnamoy10 wrote:
> > bryanpkc wrote:
> > > Can you use `llvm::sys::path::remove_filename` here?
> > Thank you.  This seems like a better option, but I could not make it work.  
> > For the sake of time I am keeping this hard coded manipulation.  It can be 
> > improved in a later revision.
> It would be really nice to use `llvm::sys::path::remove_filename` instead. 
> Perhaps something like this:
> ```
> llvm::SmallString<128> driverPath;
> driverPath.assign(llvm::sys::fs::getMainExecutable(nullptr, nullptr));
> llvm::sys::path::remove_filename(driverPath);
> ```
> ?
This works, thanks for the code!


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

https://reviews.llvm.org/D97080

___
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 331888.
arnamoy10 added a comment.

Using `llvm::sys::path::remove_filename` as per suggestion.


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/Driver/Inputs/ieee_arithmetic.mod
  flang/test/Driver/Inputs/iso_fortran_env.mod
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/intrinsic_module_path.f90

Index: flang/test/Driver/intrinsic_module_path.f90
===
--- /dev/null
+++ flang/test/Driver/intrinsic_module_path.f90
@@ -0,0 +1,37 @@
+! Ensure argument -fintrinsic-modules-path works as expected.
+! WITHOUT the option, the default location for the module is checked and no error generated.
+! With the option GIVEN, the module with the same name is PREPENDED, and considered over the
+! default one, causing a CHECKSUM error.
+
+! REQUIRES: new-flang-driver
+
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: %flang-new -fsyntax-only %s  2>&1 | FileCheck %s --allow-empty --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: %flang-new -fc1 %s  2>&1 | FileCheck %s --allow-empty --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-NOT: 'ieee_arithmetic.mod' was not found
+! WITHOUT-NOT: 'iso_fortran_env.mod' was not found
+
+!-
+! EXPECTED OUTPUT WITH
+!-
+! GIVEN: error: Cannot read module file for module 'ieee_arithmetic': File has invalid checksum
+! GIVEN: error: Cannot read module file for module 'iso_fortran_env': File has invalid checksum
+
+
+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/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -35,6 +35,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: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
@@ -83,6 +85,8 @@
 ! HELP-FC1-NEXT: -fget-symbols-sources   Dump symbols and their source code locations
 ! 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: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-FC1-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-FC1-NEXT: -fopenacc  Enable OpenACC
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -35,6 +35,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: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! CHECK-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! CHECK-NEXT: -fno-color-diagnostics Disable colors in diagnostics
Index: flang/test/Driver/Inputs/iso_fortran_env.mod
===
--- /dev/null
+++ 

[PATCH] D98867: [HIP] Fix ROCm detection

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:271
+}
+if (LatestROCm < FileName)
+  LatestROCm = FileName.str();

tra wrote:
> This will rank `rocm-3.9` higher than `rocm-3.10`. I think you do need to 
> extract and compare the version.
Good catch. Will fix it.


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

https://reviews.llvm.org/D98867

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