[PATCH] D148634: [RISCV] Bump Zfa version to 0.2 and correct RISCVUsage description

[Alex Bradbury 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 rG2a5661c84158: [RISCV] Bump Zfa version to 0.2 and correct 
RISCVUsage description (authored by asb).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148634

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/docs/ReleaseNotes.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s


Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -234,8 +234,8 @@
 .attribute arch, "rv32izifencei2p0"
 # CHECK: attribute  5, "rv32i2p1_zifencei2p0"
 
-.attribute arch, "rv32izfa0p1"
-# CHECK: attribute  5, "rv32i2p1_f2p2_zicsr2p0_zfa0p1"
+.attribute arch, "rv32izfa0p2"
+# CHECK: attribute  5, "rv32i2p1_f2p2_zicsr2p0_zfa0p2"
 
 .attribute arch, "rv32izicond1p0"
 # CHECK: attribute  5, "rv32i2p1_zicond1p0"
Index: llvm/test/CodeGen/RISCV/attributes.ll
===
--- llvm/test/CodeGen/RISCV/attributes.ll
+++ llvm/test/CodeGen/RISCV/attributes.ll
@@ -193,7 +193,7 @@
 ; RV32ZCF: .attribute 5, "rv32i2p1_zcf1p0"
 ; RV32ZICSR: .attribute 5, "rv32i2p1_zicsr2p0"
 ; RV32ZIFENCEI: .attribute 5, "rv32i2p1_zifencei2p0"
-; RV32ZFA: .attribute 5, "rv32i2p1_f2p2_zicsr2p0_zfa0p1"
+; RV32ZFA: .attribute 5, "rv32i2p1_f2p2_zicsr2p0_zfa0p2"
 ; RV32ZVKB: .attribute 5, 
"rv32i2p1_zicsr2p0_zve32x1p0_zve64x1p0_zvkb0p3_zvl32b1p0_zvl64b1p0"
 ; RV32ZVKG: .attribute 5, "rv32i2p1_zicsr2p0_zve32x1p0_zvkg0p3_zvl32b1p0"
 ; RV32ZVKN: .attribute 5, 
"rv32i2p1_zicsr2p0_zve32x1p0_zve64x1p0_zvkb0p3_zvkn0p3_zvkned0p3_zvknha0p3_zvknhb0p3_zvl32b1p0_zvl64b1p0"
@@ -264,7 +264,7 @@
 ; RV64ZCD: .attribute 5, "rv64i2p1_zcd1p0"
 ; RV64ZICSR: .attribute 5, "rv64i2p1_zicsr2p0"
 ; RV64ZIFENCEI: .attribute 5, "rv64i2p1_zifencei2p0"
-; RV64ZFA: .attribute 5, "rv64i2p1_f2p2_zicsr2p0_zfa0p1"
+; RV64ZFA: .attribute 5, "rv64i2p1_f2p2_zicsr2p0_zfa0p2"
 ; RV64ZVKB: .attribute 5, 
"rv64i2p1_zicsr2p0_zve32x1p0_zve64x1p0_zvkb0p3_zvl32b1p0_zvl64b1p0"
 ; RV64ZVKG: .attribute 5, "rv64i2p1_zicsr2p0_zve32x1p0_zvkg0p3_zvl32b1p0"
 ; RV64ZVKN: .attribute 5, 
"rv64i2p1_zicsr2p0_zve32x1p0_zve64x1p0_zvkb0p3_zvkn0p3_zvkned0p3_zvknha0p3_zvknhb0p3_zvl32b1p0_zvl64b1p0"
Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -139,7 +139,7 @@
 {"zcb", RISCVExtensionVersion{1, 0}},
 {"zcd", RISCVExtensionVersion{1, 0}},
 {"zcf", RISCVExtensionVersion{1, 0}},
-{"zfa", RISCVExtensionVersion{0, 1}},
+{"zfa", RISCVExtensionVersion{0, 2}},
 {"zicond", RISCVExtensionVersion{1, 0}},
 {"zvfh", RISCVExtensionVersion{0, 1}},
 {"ztso", RISCVExtensionVersion{0, 1}},
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -166,6 +166,7 @@
   on RISC-V, but conforms with the new "platform register" defined in the
   RISC-V psABI (for more details see the 
   `psABI discussion 
`_).
+* Added support for Zfa extension version 0.2.
 
 Changes to the WebAssembly Backend
 --
Index: llvm/docs/RISCVUsage.rst
===
--- llvm/docs/RISCVUsage.rst
+++ llvm/docs/RISCVUsage.rst
@@ -188,7 +188,7 @@
   LLVM implements the `1.0.1 draft specification 
`_.
 
 ``experimental-zfa``
-  LLVM implements a subset of `0.1 draft specification 
`_
 (see Chapter 25).
+  LLVM implements the `0.2 draft specification 
`_.
 
 ``experimental-zicond``
   LLVM implements the `1.0-rc1 draft specification 
_.
Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -518,12 +518,12 @@
 // CHECK-ZIFENCEI-EXT: __riscv_zifencei 200{{$}}
 
 // RUN: %clang -target riscv32-unknown-linux-gnu 
-menable-experimental-extensions \
-// RUN: -march=rv32izfa0p1 -x c -E -dM %s \
+// RUN: -march=rv32izfa0p2 -x c -E -dM %s \
 // RUN: 

[clang] 2a5661c - [RISCV] Bump Zfa version to 0.2 and correct RISCVUsage description

[Alex Bradbury via cfe-commits]

Author: Alex Bradbury
Date: 2023-04-19T06:27:35+01:00
New Revision: 2a5661c8415876be3fbd56ce90c2031e89ba0ef3

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

LOG: [RISCV] Bump Zfa version to 0.2 and correct RISCVUsage description

As of
https://github.com/riscv/riscv-isa-manual/commit/1f038182810727f5feca311072e630d6baac51da
in the riscv-isa-manual, Zfa is at version 0.2. Reviewing the commit
history for
zfa.tex

there are no relevant changes since 0.1. As such, we can simply
increment the version number.

This change also removes the claim in RISCVUsage that we implement a
"subset of" Zfa, as I believe this is no longer true. That sentence
previously incorrectly claimed we didn't implement fli.{h,s,d} (I
[corrected this a couple of weeks
ago](https://reviews.llvm.org/rG3d969191b277)) but I think should have
removed the "subset of" wording too.

As was noted during the review, we never added Zfa to the release notes.
This is corrected in this patch.

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

Added: 


Modified: 
clang/test/Preprocessor/riscv-target-features.c
llvm/docs/RISCVUsage.rst
llvm/docs/ReleaseNotes.rst
llvm/lib/Support/RISCVISAInfo.cpp
llvm/test/CodeGen/RISCV/attributes.ll
llvm/test/MC/RISCV/attribute-arch.s

Removed: 




diff  --git a/clang/test/Preprocessor/riscv-target-features.c 
b/clang/test/Preprocessor/riscv-target-features.c
index 1bd10e96f15af..79b7af954898d 100644
--- a/clang/test/Preprocessor/riscv-target-features.c
+++ b/clang/test/Preprocessor/riscv-target-features.c
@@ -518,12 +518,12 @@
 // CHECK-ZIFENCEI-EXT: __riscv_zifencei 200{{$}}
 
 // RUN: %clang -target riscv32-unknown-linux-gnu 
-menable-experimental-extensions \
-// RUN: -march=rv32izfa0p1 -x c -E -dM %s \
+// RUN: -march=rv32izfa0p2 -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-ZFA-EXT %s
 // RUN: %clang -target riscv64-unknown-linux-gnu 
-menable-experimental-extensions \
-// RUN: -march=rv64izfa0p1 -x c -E -dM %s \
+// RUN: -march=rv64izfa0p2 -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-ZFA-EXT %s
-// CHECK-ZFA-EXT: __riscv_zfa 1000{{$}}
+// CHECK-ZFA-EXT: __riscv_zfa 2000{{$}}
 
 // RUN: %clang -target riscv32 -menable-experimental-extensions \
 // RUN: -march=rv32i_zve64x_zvkb0p3 -x c -E -dM %s \

diff  --git a/llvm/docs/RISCVUsage.rst b/llvm/docs/RISCVUsage.rst
index e9a786eaa4146..c9ccf3c3bd0a6 100644
--- a/llvm/docs/RISCVUsage.rst
+++ b/llvm/docs/RISCVUsage.rst
@@ -188,7 +188,7 @@ The primary goal of experimental support is to assist in 
the process of ratifica
   LLVM implements the `1.0.1 draft specification 
`_.
 
 ``experimental-zfa``
-  LLVM implements a subset of `0.1 draft specification 
`_
 (see Chapter 25).
+  LLVM implements the `0.2 draft specification 
`_.
 
 ``experimental-zicond``
   LLVM implements the `1.0-rc1 draft specification 
_.

diff  --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 451db539cfe66..7678a29853373 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -166,6 +166,7 @@ Changes to the RISC-V Backend
   on RISC-V, but conforms with the new "platform register" defined in the
   RISC-V psABI (for more details see the 
   `psABI discussion 
`_).
+* Added support for Zfa extension version 0.2.
 
 Changes to the WebAssembly Backend
 --

diff  --git a/llvm/lib/Support/RISCVISAInfo.cpp 
b/llvm/lib/Support/RISCVISAInfo.cpp
index 7cfadec082fda..f3f53db574472 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -139,7 +139,7 @@ static const RISCVSupportedExtension 
SupportedExperimentalExtensions[] = {
 {"zcb", RISCVExtensionVersion{1, 0}},
 {"zcd", RISCVExtensionVersion{1, 0}},
 {"zcf", RISCVExtensionVersion{1, 0}},
-{"zfa", RISCVExtensionVersion{0, 1}},
+{"zfa", RISCVExtensionVersion{0, 2}},
 {"zicond", RISCVExtensionVersion{1, 0}},
 {"zvfh", RISCVExtensionVersion{0, 1}},
 {"ztso", RISCVExtensionVersion{0, 1}},

diff  --git a/llvm/test/CodeGen/RISCV/attributes.ll 
b/llvm/test/CodeGen/RISCV/attributes.ll
index ea978ee39b5e1..83ee6cae07b2a 100644
--- a/llvm/test/CodeGen/RISCV/attributes.ll
+++ b/llvm/test/CodeGen/RISCV/attributes.ll
@@ -193,7 +193,7 @@
 ; RV32ZCF: .attribute 5, 

[PATCH] D148223: [SiFive] Support C intrinsics for xsfvcp extension.

[Brandon Wu via Phabricator via cfe-commits]

4vtomat updated this revision to Diff 514821.
4vtomat added a comment.

Resolved Craig's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148223

Files:
  clang/include/clang/Basic/riscv_sifive_vector.td
  clang/include/clang/Basic/riscv_vector.td
  clang/lib/Sema/SemaChecking.cpp
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-x-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-x.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xv-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvv-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvw.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/xsfvcp-index-out-of-range.c

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

[PATCH] D148680: [RISCV] Split out part of riscv_vector.td to riscv_vector_common.td

[Brandon Wu via Phabricator via cfe-commits]

4vtomat added a comment.

In D148680#4279291 , @craig.topper 
wrote:

> Are we running tablegen separately for each vendor extension?

Yes, I guess the vendor extensions should be separated into different tablegen 
file so that they don't be in risv_vector.h, instead the customers should 
include vendor's header once needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148680

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

[PATCH] D133863: [RISCV] Add MC support of RISCV zcmt Extension

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Support/RISCVISAInfo.cpp:903
+return createStringError(errc::invalid_argument,
+ "zcmt is not allowed when c is specified");
+

Extension names should be in single quotes.

Should we use `'zcmt' and 'c' extensions are incompatible` to match line 863?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133863

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

[PATCH] D148223: [SiFive] Support C intrinsics for xsfvcp extension.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_sifive_vector.td:41
+  if !find(prototype, "0") then {
+  def : VCIXBuiltinSet;
+  }

Indent



Comment at: clang/include/clang/Basic/riscv_sifive_vector.td:64
+  // lots of redundant intrinsic but have same names.
+ let Log2LMUL = [0] in
+def : VCIXBuiltinSet;

This should be indented further


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148223

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

[PATCH] D147307: [clang] Do not require GNUInlineAttr for inline builtins

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added a comment.

This change seems cause assert:

https://godbolt.org/z/z9G87Wr37


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147307

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

[PATCH] D148601: [Clang] Handle Error message to output proper Prefix

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

tbaeder added inline comments.



Comment at: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp:1097
 // source file(s) processed.
+const auto  = Diags.getDiagnosticOptions().VerifyPrefixes;
 if (Status == HasNoDirectives) {

Can move this line inside the `if` statement block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148601

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

[PATCH] D148601: [Clang] Handle Error message to output proper Prefix

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

tbaeder added a comment.

I am not 100% sure about the semantics of passing multiple prefixes, i.e. if 
the error is emitted for all prefixes individually or if it's only emitted if 
no `expected` line for any of the prefixes is found. In the latter case we 
should probably add all the prefixes to the error message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148601

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

[PATCH] D148308: [RISCV] Split out SiFive VCIX C intrinsics from riscv_vector.td

[Brandon Wu via Phabricator via cfe-commits]

4vtomat updated this revision to Diff 514817.
4vtomat added a comment.

Resolved Kito's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148308

Files:
  clang/include/clang/Basic/BuiltinsRISCVVector.def
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/riscv_sifive_vector.td
  clang/include/clang/Basic/riscv_vector.td
  clang/include/clang/Sema/RISCVIntrinsicManager.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/sifive_vector.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaRISCVVectorLookup.cpp
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-x-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-x.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xv-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvv-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvw.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/xsfvcp-index-out-of-range.c
  clang/test/Sema/riscv-bad-intrinsic-pragma.c
  clang/utils/TableGen/TableGen.cpp
  llvm/docs/CommandGuide/tblgen.rst

Index: llvm/docs/CommandGuide/tblgen.rst
===
--- llvm/docs/CommandGuide/tblgen.rst
+++ llvm/docs/CommandGuide/tblgen.rst
@@ -348,6 +348,14 @@
 
   Generate ``riscv_vector_builtin_cg.inc`` for Clang.
 
+.. option:: -gen-riscv-sifive-vector-builtins
+
+  Generate ``riscv_sifive_vector_builtins.inc`` for Clang.
+
+.. option:: -gen-riscv-sifive-vector-builtin-codegen
+
+  Generate ``riscv_sifive_vector_builtin_cg.inc`` for Clang.
+
 .. option:: -gen-attr-docs
 
   Generate attribute documentation.
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -91,6 +91,9 @@
   GenRISCVVectorBuiltins,
   GenRISCVVectorBuiltinCG,
   GenRISCVVectorBuiltinSema,
+  GenRISCVSiFiveVectorBuiltins,
+  GenRISCVSiFiveVectorBuiltinCG,
+  GenRISCVSiFiveVectorBuiltinSema,
   GenAttrDocs,
   GenDiagDocs,
   GenOptDocs,
@@ -251,6 +254,12 @@
"Generate riscv_vector_builtin_cg.inc for clang"),
 clEnumValN(GenRISCVVectorBuiltinSema, "gen-riscv-vector-builtin-sema",
"Generate riscv_vector_builtin_sema.inc for clang"),
+clEnumValN(GenRISCVSiFiveVectorBuiltins, "gen-riscv-sifive-vector-builtins",
+   "Generate riscv_sifive_vector_builtins.inc for clang"),
+clEnumValN(GenRISCVSiFiveVectorBuiltinCG, "gen-riscv-sifive-vector-builtin-codegen",
+   "Generate riscv_sifive_vector_builtin_cg.inc for clang"),
+clEnumValN(GenRISCVSiFiveVectorBuiltinSema, "gen-riscv-sifive-vector-builtin-sema",
+   "Generate riscv_sifive_vector_builtin_sema.inc for clang"),
 clEnumValN(GenAttrDocs, "gen-attr-docs",
"Generate attribute documentation"),
 clEnumValN(GenDiagDocs, "gen-diag-docs",
@@ -472,6 +481,15 @@
   case GenRISCVVectorBuiltinSema:
 EmitRVVBuiltinSema(Records, OS);
 break;
+  case GenRISCVSiFiveVectorBuiltins:
+EmitRVVBuiltins(Records, OS);
+break;
+  case GenRISCVSiFiveVectorBuiltinCG:
+EmitRVVBuiltinCG(Records, OS);
+break;
+  case GenRISCVSiFiveVectorBuiltinSema:
+EmitRVVBuiltinSema(Records, OS);
+break;
   case GenAttrDocs:
 EmitClangAttrDocs(Records, OS);
 break;
Index: clang/test/Sema/riscv-bad-intrinsic-pragma.c
===
--- clang/test/Sema/riscv-bad-intrinsic-pragma.c
+++ clang/test/Sema/riscv-bad-intrinsic-pragma.c
@@ -2,7 +2,7 @@
 // RUN:2>&1 | FileCheck %s
 
 #pragma clang riscv intrinsic 
-// CHECK:  warning: unexpected argument '' to '#pragma riscv'; expected 'vector' [-Wignored-pragmas]
+// CHECK:  warning: unexpected argument '' to '#pragma riscv'; expected 'vector' or 'sifive_vector' [-Wignored-pragmas]
 
 #pragma clang riscv what + 3241
 // CHECK:  warning: unexpected argument 'what' to '#pragma riscv'; expected 'intrinsic' [-Wignored-pragmas]
Index: clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/xsfvcp-index-out-of-range.c
===
--- clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/xsfvcp-index-out-of-range.c
+++ clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/xsfvcp-index-out-of-range.c
@@ -5,7 

[PATCH] D148680: [RISCV] Split out part of riscv_vector.td to riscv_vector_common.td

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Are we running tablegen separately for each vendor extension?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148680

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

[PATCH] D133863: [RISCV] Add MC support of RISCV zcmt Extension

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added inline comments.



Comment at: lld/ELF/InputSection.h:404
 
-static_assert(sizeof(InputSection) <= 152, "InputSection is too big");
+static_assert(sizeof(InputSection) <= 160, "InputSection is too big");
 

Why we need this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133863

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

[PATCH] D148223: [SiFive] Support C intrinsics for xsfvcp extension.

[Brandon Wu via Phabricator via cfe-commits]

4vtomat updated this revision to Diff 514812.
4vtomat added a comment.

Rename riscv_sifive_vcix.td to riscv_sifive_vector.td


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148223

Files:
  clang/include/clang/Basic/riscv_sifive_vector.td
  clang/include/clang/Basic/riscv_vector.td
  clang/lib/Sema/SemaChecking.cpp
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-x-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-x.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xv-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvv-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvw.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/xsfvcp-index-out-of-range.c

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

[PATCH] D148223: [SiFive] Support C intrinsics for xsfvcp extension.

[Brandon Wu via Phabricator via cfe-commits]

4vtomat updated this revision to Diff 514810.
4vtomat added a comment.

Resolved Kito's comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148223

Files:
  clang/include/clang/Basic/riscv_sifive_vcix.td
  clang/include/clang/Basic/riscv_vector.td
  clang/lib/Sema/SemaChecking.cpp
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-x-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-x.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xv-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvv-rv64.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/xsfvcp-xvw.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/xsfvcp-index-out-of-range.c

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

[PATCH] D148680: [RISCV] Split out part of riscv_vector.td to riscv_vector_common.td

[Brandon Wu via Phabricator via cfe-commits]

4vtomat created this revision.
Herald added subscribers: jobnoorman, luke, VincentWu, vkmr, frasercrmck, 
evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, 
jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, 
zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, 
rbar, asb, arichardson.
Herald added a project: All.
4vtomat requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD.
Herald added a project: clang.

This makes other new targets able to reuse predefined classes
in their own *.td files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148680

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/include/clang/Basic/riscv_vector_common.td

Index: clang/include/clang/Basic/riscv_vector_common.td
===
--- /dev/null
+++ clang/include/clang/Basic/riscv_vector_common.td
@@ -0,0 +1,239 @@
+//==-- riscv_vector_common.td - RISC-V V-ext builtin class ===//
+//
+//  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
+//
+//===--===//
+//
+// This file defines RVV builtin base class for RISC-V V-extension.
+//
+//===--===//
+
+//===--===//
+// Instruction definitions
+//===--===//
+// Each record of the class RVVBuiltin defines a collection of builtins (i.e.
+// "def vadd : RVVBuiltin" will be used to define things like "vadd_vv_i32m1",
+// "vadd_vv_i32m2", etc).
+//
+// The elements of this collection are defined by an instantiation process the
+// range of which is specified by the cross product of the LMUL attribute and
+// every element in the attribute TypeRange. By default builtins have LMUL = [1,
+// 2, 4, 8, 1/2, 1/4, 1/8] so the process is repeated 7 times. In tablegen we
+// use the Log2LMUL [0, 1, 2, 3, -1, -2, -3] to represent the LMUL.
+//
+// LMUL represents the fact that the types of values used by that builtin are
+// values generated by instructions that are executed under that LMUL. However,
+// this does not mean the builtin is necessarily lowered into an instruction
+// that executes under the specified LMUL. An example where this happens are
+// loads and stores of masks. A mask like `vbool8_t` can be generated, for
+// instance, by comparing two `__rvv_int8m1_t` (this is LMUL=1) or comparing two
+// `__rvv_int16m2_t` (this is LMUL=2). The actual load or store, however, will
+// be performed under LMUL=1 because mask registers are not grouped.
+//
+// TypeRange is a non-empty sequence of basic types:
+//
+//   c: int8_t (i8)
+//   s: int16_t (i16)
+//   i: int32_t (i32)
+//   l: int64_t (i64)
+//   x: float16_t (half)
+//   f: float32_t (float)
+//   d: float64_t (double)
+//
+// This way, given an LMUL, a record with a TypeRange "sil" will cause the
+// definition of 3 builtins. Each type "t" in the TypeRange (in this example
+// they are int16_t, int32_t, int64_t) is used as a parameter that drives the
+// definition of that particular builtin (for the given LMUL).
+//
+// During the instantiation, types can be transformed or modified using type
+// transformers. Given a type "t" the following primitive type transformers can
+// be applied to it to yield another type.
+//
+//   e: type of "t" as is (identity)
+//   v: computes a vector type whose element type is "t" for the current LMUL
+//   w: computes a vector type identical to what 'v' computes except for the
+//  element type which is twice as wide as the element type of 'v'
+//   q: computes a vector type identical to what 'v' computes except for the
+//  element type which is four times as wide as the element type of 'v'
+//   o: computes a vector type identical to what 'v' computes except for the
+//  element type which is eight times as wide as the element type of 'v'
+//   m: computes a vector type identical to what 'v' computes except for the
+//  element type which is bool
+//   0: void type, ignores "t"
+//   z: size_t, ignores "t"
+//   t: ptrdiff_t, ignores "t"
+//   u: unsigned long, ignores "t"
+//   l: long, ignores "t"
+//
+// So for instance if t is "i", i.e. int, then "e" will yield int again. "v"
+// will yield an RVV vector type (assume LMUL=1), so __rvv_int32m1_t.
+// Accordingly "w" would yield __rvv_int64m2_t.
+//
+// A type transformer can be prefixed by other non-primitive type transformers.
+//
+//   P: constructs a pointer to the current type
+//   C: adds const to the type
+//   K: requires the integer type to be a constant expression
+//   U: given an integer type 

[PATCH] D146269: MIPS: allow o32 abi with 64bit CPU and 64 abi with 32bit triple

[Fangrui Song via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/Mips/MipsSubtarget.cpp:107
   // Check if Architecture and ABI are compatible.
-  assert(((!isGP64bit() && isABI_O32()) ||
-  (isGP64bit() && (isABI_N32() || isABI_N64( &&
+  assert(((!isGP64bit() && isABI_O32()) || (isGP64bit())) &&
  "Invalid  Arch & ABI pair.");




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

https://reviews.llvm.org/D146269

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

[PATCH] D146269: MIPS: allow o32 abi with 64bit CPU and 64 abi with 32bit triple

[YunQiang Su via Phabricator via cfe-commits]

wzssyqa added a comment.

ping...


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

https://reviews.llvm.org/D146269

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

[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:2332
+
+  #if __RISCV_RVV_VLEN_BITS==512
+  typedef vint8m1_t fixed_vint8m1_t 
__attribute__((riscv_rvv_vector_bits(512)));

rjmccall wrote:
> craig.topper wrote:
> > rjmccall wrote:
> > > This probably needs a `defined(__RISCV_RVV_VLEN_BITS)` clause, right?  
> > > Because the compiler doesn't actually define this macro unless 
> > > `-mrvv-vector-bits` is given.
> > I guess so. I copied the documentation from the SVE attribute and modified 
> > it to RISC-V.
> Ah, I see.  Yeah, it's probably wrong there, too.
As written the #if would evaluate to false if __RISCV_RVV_VLEN_BITS isn't 
defined or it's not defined to be 512. The code line it was guarding is using a 
hardcoded 512.

This isn't how I'd encourage this to be used so I'm changing to #if defined() 
and will use the preprocessor define in the next line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145088

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

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

This patch is causing breakages downstream because it didn't have do the struct 
dance, so I've added D148677 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

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

[PATCH] D148677: [clang] makes `__is_trivially_equality_comparable` available as a struct

[Christopher Di Bella via Phabricator via cfe-commits]

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

uploads the right test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148677

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp


Index: clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify 
-std=gnu++20 -fms-extensions -Wno-microsoft %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify 
-std=gnu++2b -fms-extensions -Wno-microsoft %s
+
+template 
+struct Same {
+  static constexpr auto value = __is_same(T, U);
+};
+
+template 
+struct __is_trivially_equality_comparable { // expected-warning{{keyword 
'__is_trivially_equality_comparable' will be made available as an identifier 
for the remainder of the translation unit}}
+  using type = T;
+};
+
+using A = Same<__is_trivially_equality_comparable::type, 
__is_trivially_equality_comparable::type>;
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -1622,6 +1622,7 @@
   tok::kw___is_signed,
   tok::kw___is_standard_layout,
   tok::kw___is_trivial,
+  tok::kw___is_trivially_equality_comparable,
   tok::kw___is_trivially_assignable,
   tok::kw___is_trivially_constructible,
   tok::kw___is_trivially_copyable,


Index: clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++20 -fms-extensions -Wno-microsoft %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++2b -fms-extensions -Wno-microsoft %s
+
+template 
+struct Same {
+  static constexpr auto value = __is_same(T, U);
+};
+
+template 
+struct __is_trivially_equality_comparable { // expected-warning{{keyword '__is_trivially_equality_comparable' will be made available as an identifier for the remainder of the translation unit}}
+  using type = T;
+};
+
+using A = Same<__is_trivially_equality_comparable::type, __is_trivially_equality_comparable::type>;
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -1622,6 +1622,7 @@
   tok::kw___is_signed,
   tok::kw___is_standard_layout,
   tok::kw___is_trivial,
+  tok::kw___is_trivially_equality_comparable,
   tok::kw___is_trivially_assignable,
   tok::kw___is_trivially_constructible,
   tok::kw___is_trivially_copyable,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148677: [clang] makes `__is_trivially_equality_comparable` available as a struct

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb created this revision.
cjdb added reviewers: aaron.ballman, erichkeane, shafik, dblaikie.
Herald added a project: All.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Since this was originally a name in library, it needs an escape hatch
for versions of Clang that are slightly out-of-sync with libc++.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148677

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/SemaCXX/move-not-noexcept.cpp


Index: clang/test/SemaCXX/move-not-noexcept.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/move-not-noexcept.cpp
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+struct TrivialMoveOps {};
+
+struct DefaultedMoveCtor {
+  DefaultedMoveCtor(DefaultedMoveCtor&&) = default;
+};
+
+struct DefaultedMoveAssign {
+  DefaultedMoveAssign& operator=(DefaultedMoveAssign&&) = default;
+};
+
+struct UnknownNoexceptCtor {
+  UnknownNoexceptCtor(UnknownNoexceptCtor&&);
+  // expected-warning@-1{{move constructor for 'UnknownNoexceptCtor' is not 
noexcept, which can cause performance issues; use 'noexcept(false)' if this is 
intentional}}
+};
+
+struct UnknownNoexceptAssign {
+  UnknownNoexceptAssign& operator=(UnknownNoexceptAssign&&);
+  // expected-warning@-1{{move assignment operator for 'UnknownNoexceptAssign' 
is not noexcept, which can cause performance issues; use 'noexcept(false)' if 
this is intentional}}
+};
+
+struct NoexceptSpecifierCtor {
+  NoexceptSpecifierCtor(NoexceptSpecifierCtor&&) noexcept;
+};
+
+struct NoexceptSpecifierAssign {
+  NoexceptSpecifierAssign& operator=(NoexceptSpecifierAssign&&) noexcept;
+};
+
+struct NoexceptTrueSpecifierCtor {
+  NoexceptTrueSpecifierCtor(NoexceptTrueSpecifierCtor&&) noexcept(true);
+};
+
+struct NoexceptTrueSpecifierAssign {
+  NoexceptTrueSpecifierAssign& operator=(NoexceptTrueSpecifierAssign&&) 
noexcept(true);
+};
+
+struct NoexceptFalseSpecifierCtor {
+  NoexceptFalseSpecifierCtor(NoexceptFalseSpecifierCtor&&) noexcept(false);
+};
+
+struct NoexceptFalseSpecifierAssign {
+  NoexceptFalseSpecifierAssign& operator=(NoexceptFalseSpecifierAssign&&) 
noexcept(false);
+};
+
+template
+struct NoexceptDependentSpecifierCtor {
+  NoexceptDependentSpecifierCtor(NoexceptDependentSpecifierCtor&&) 
noexcept(__is_integral(T));
+};
+
+template
+struct NoexceptDependentSpecifierAssign {
+  NoexceptDependentSpecifierAssign& 
operator=(NoexceptDependentSpecifierAssign&&) noexcept(__is_integral(T));
+};
+
+struct ThrowParensSpecifierCtor {
+  ThrowParensSpecifierCtor(ThrowParensSpecifierCtor&&) throw();
+};
+
+struct ThrowParensSpecifierAssign {
+  ThrowParensSpecifierAssign& operator=(ThrowParensSpecifierAssign&&) throw();
+};
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -1622,6 +1622,7 @@
   tok::kw___is_signed,
   tok::kw___is_standard_layout,
   tok::kw___is_trivial,
+  tok::kw___is_trivially_equality_comparable,
   tok::kw___is_trivially_assignable,
   tok::kw___is_trivially_constructible,
   tok::kw___is_trivially_copyable,


Index: clang/test/SemaCXX/move-not-noexcept.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/move-not-noexcept.cpp
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+struct TrivialMoveOps {};
+
+struct DefaultedMoveCtor {
+  DefaultedMoveCtor(DefaultedMoveCtor&&) = default;
+};
+
+struct DefaultedMoveAssign {
+  DefaultedMoveAssign& operator=(DefaultedMoveAssign&&) = default;
+};
+
+struct UnknownNoexceptCtor {
+  UnknownNoexceptCtor(UnknownNoexceptCtor&&);
+  // expected-warning@-1{{move constructor for 'UnknownNoexceptCtor' is not noexcept, which can cause performance issues; use 'noexcept(false)' if this is intentional}}
+};
+
+struct UnknownNoexceptAssign {
+  UnknownNoexceptAssign& operator=(UnknownNoexceptAssign&&);
+  // expected-warning@-1{{move assignment operator for 'UnknownNoexceptAssign' is not noexcept, which can cause performance issues; use 'noexcept(false)' if this is intentional}}
+};
+
+struct NoexceptSpecifierCtor {
+  NoexceptSpecifierCtor(NoexceptSpecifierCtor&&) noexcept;
+};
+
+struct NoexceptSpecifierAssign {
+  NoexceptSpecifierAssign& operator=(NoexceptSpecifierAssign&&) noexcept;
+};
+
+struct NoexceptTrueSpecifierCtor {
+  NoexceptTrueSpecifierCtor(NoexceptTrueSpecifierCtor&&) noexcept(true);
+};
+
+struct NoexceptTrueSpecifierAssign {
+  NoexceptTrueSpecifierAssign& operator=(NoexceptTrueSpecifierAssign&&) noexcept(true);
+};
+
+struct NoexceptFalseSpecifierCtor {
+  NoexceptFalseSpecifierCtor(NoexceptFalseSpecifierCtor&&) noexcept(false);
+};
+
+struct NoexceptFalseSpecifierAssign {
+  NoexceptFalseSpecifierAssign& 

[PATCH] D147901: [NFC][CLANG][API] Fix coverity remarks about large copies by values

[Soumi Manna via Phabricator via cfe-commits]

Manna added a comment.

Thank you everyone for reviews and feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147901

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

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:263
 
+  if (SubstitutedAtomicExpr.get()->isValueDependent())
+return SubstitutedAtomicExpr;

alexander-shaposhnikov wrote:
> alexander-shaposhnikov wrote:
> > erichkeane wrote:
> > > alexander-shaposhnikov wrote:
> > > > erichkeane wrote:
> > > > > alexander-shaposhnikov wrote:
> > > > > > erichkeane wrote:
> > > > > > > So this bit is concerning to me... we have been catching a ton of 
> > > > > > > bugs in the MLTAL stuff by trying to constant evaluate an 
> > > > > > > expression that isn't correctly constexpr, and this will defeat 
> > > > > > > it.  We shouldn't be calling this function at all on 
> > > > > > > non-fully-instantiated expressions.  What is the case that ends 
> > > > > > > up coming through here, and should be be calling this at all?
> > > > > > This happens e.g. for concepts-PR54629.cpp
> > > > > > 
> > > > > > ```
> > > > > > Old:
> > > > > > FunctionDecl 0x64d90378 
> > > > > > llvm-project/clang/test/SemaTemplate/concepts-PR54629.cpp:30:1, 
> > > > > > line:32:2> line:30:6 foo 'void ()'
> > > > > > |-RequiresExpr 0x64d90318  'bool'
> > > > > > | |-ParmVarDecl 0x64d90150  col:24 referenced t 
> > > > > > 'T &'
> > > > > > | `-NestedRequirement 0x64d902d8 dependent
> > > > > > |   `-BinaryOperator 0x64d902b8  'bool' '<'
> > > > > > | |-UnaryExprOrTypeTraitExpr 0x64d90260  
> > > > > > 'unsigned long' sizeof
> > > > > > | | `-ParenExpr 0x64d90240  'T' lvalue
> > > > > > | |   `-DeclRefExpr 0x64d90220  'T' lvalue ParmVar 
> > > > > > 0x64d90150 't' 'T &' non_odr_use_unevaluated
> > > > > > | `-ImplicitCastExpr 0x64d902a0  'unsigned long' 
> > > > > > 
> > > > > > |   `-IntegerLiteral 0x64d90280  'int' 4
> > > > > > `-CompoundStmt 0x64d90f70 
> > > > > > 
> > > > > > while MLTAL is empty.
> > > > > > ```
> > > > > > (clang::Sema::CheckOverload calls clang::Sema::IsOverload, while 
> > > > > > clang::Sema::IsOverload invokes 
> > > > > > clang::Sema::AreConstraintExpressionsEqual)
> > > > > Hmm that seems wrong for me, but I'm not sure how.  It doesn't 
> > > > > seem right for `AreConstraintExpressionsEqual`to try to calculate the 
> > > > > constraint satisfaction...
> > > > I think we get there from 
> > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaTemplateInstantiate.cpp#L2375
> > > So I still think this is an incorrect change.  I don't understand why 
> > > we'd get here without the 'final' check, but perhaps there is something 
> > > missing elsewhere?
> > unfortunately, I don't have enough context - will revisit this change (iirc 
> > one test was failing without it and Clang was calling 
> > CheckConstraintSatisfaction from an unexpected place). There is some 
> > accumulated technical debt (without the current diff) / it takes to 
> > untangle.  
> reduced test case (extracted from concepts-PR54629.cpp):
> 
> ```
> template 
> void foo()
>   requires requires(T ) { requires sizeof(t) < 4; }
> {}
> 
> template 
> void foo()
>   requires requires(T ) { requires sizeof(t) > 4; }
> {}
> 
> void test() {
>   foo();
> }
> ```
so I've investigated a bit how things work right now (without this diff),
the following code is somewhat (but not directly) relevant:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaConcept.cpp#L417
 
(from 
https://github.com/llvm/llvm-project/commit/b25902736c2e330429278e1929cc5afd2201fb77)
 
however, previously, the issues above (for the previous version of this diff) 
(e.g. for concepts-PR54629.cpp) were triggered when Clang was trying to 
substitute an empty MLTAL, while in our case it's unnecessary. 
It might happen though that there is another problem lingering around but it 
looks like at the moment the tests are passing without changes to 
```
calculateConstraintSatisfaction
```
(=> dropped them)
 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

[Guillot Tony via Phabricator via cfe-commits]

to268 updated this revision to Diff 514789.
to268 added a comment.

I have *hopefully* fixed my broken patch.
This is all the things in need to address:

  auto str2[] = "string";
  [[clang::overloadable]] auto test(auto x) { return x; }

and maybe retry to support `auto` in compound literals,
becaue i have seen heavy macro users that want compound literals to work.

I think it's safe to say that we will not support all features from N3076 
 in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/C/C2x/n3007.c
  clang/test/CodeGen/auto.c
  clang/test/Parser/c2x-auto.c
  clang/test/Sema/c2x-auto.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -1195,7 +1195,7 @@
 
   Type inference for object declarations
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3007.htm;>N3007
-  No
+  Clang 17
 
 
   constexpr for object definitions
Index: clang/test/Sema/c2x-auto.c
===
--- /dev/null
+++ clang/test/Sema/c2x-auto.c
@@ -0,0 +1,76 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c2x %s
+
+void test_basic_types(void) {
+  auto undefined; // expected-error {{declaration of variable 'undefined' with deduced type 'auto' requires an initializer}}
+  auto auto_int = 4;
+  auto auto_long = 4UL;
+}
+
+void test_sizeof_typeof(void) {
+  auto auto_size = sizeof(auto);  // expected-error {{expected expression}}
+  typeof(auto) tpof = 4;  // expected-error {{expected expression}}
+}
+
+void test_casts(void) {
+  auto int_cast = (int)(4 + 3);
+  auto double_cast = (double)(1 / 3);
+  auto long_cast = (long)(4UL + 3UL);
+  auto auto_cast = (auto)(4 + 3); // expected-error {{expected expression}}
+}
+
+void test_compound_literral(void) {
+  auto int_cl = (int){13};
+  auto double_cl = (double){2.5};
+  auto array[] = { 1, 2, 3 }; // expected-error {{cannot use 'auto' with initializer list in C}}
+
+  // FIXME: This should be accepted per C2x 6.5.2.5p4
+  auto auto_cl = (auto){13};  // expected-error {{expected expression}}
+}
+
+void test_array_pointers(void) {
+  double array[3] = { 0 };
+  auto a = array;
+  auto b = 
+}
+
+void test_qualifiers(const int y) {
+  const auto a = 12;
+  auto b = y;
+  static auto c = 1UL;
+  int* pa =  // expected-warning {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}}
+  const int* pb = 
+  int* pc =  // expected-warning {{incompatible pointer types initializing 'int *' with an expression of type 'unsigned long *'}}
+
+}
+
+void test_strings(void) {
+  auto str = "this is a string";
+  // FIXME: This should work for char*
+  auto str2[] = "this is a string"; // expected-error {{variable 'str2' with type 'auto[]' has incompatible initializer of type 'char[17]'}}
+  auto (str3) = "this is a string";
+  auto (((str4))) = "this is a string";
+}
+
+void test_pointers(void) {
+  // FIXME: as mentioned in N3076, it seems that auto* is allowed
+  auto a = 12;
+  auto *ptr = 
+  auto *str = "this is a string";
+  const auto *str2 = "this is a string";
+  auto *b = 
+  *b =  // expected-error {{incompatible pointer to integer conversion assigning to 'int' from 'int *'; remove &}}
+  auto nptr = nullptr;
+}
+
+void test_scopes(void) {
+  double a = 7;
+  double b = 9;
+  {
+auto a = a * a; // expected-error {{variable 'a' declared with deduced type 'auto' cannot appear in its own initializer}} \
+   expected-error {{variable 'a' declared with deduced type 'auto' cannot appear in its own initializer}}
+  }
+  {
+auto b = a * a;
+auto a = b;
+  }
+}
Index: clang/test/Parser/c2x-auto.c
===
--- /dev/null
+++ clang/test/Parser/c2x-auto.c
@@ -0,0 +1,123 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c2x -std=c2x %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c17 -std=c17 %s
+
+#define AUTO_MACRO(_NAME, ARG, ARG2, ARG3) \
+auto _NAME = ARG + (ARG2 / ARG3);
+
+struct S {
+  int a;
+  auto b;   // c2x-error {{'auto' not allowed in struct member}} \
+   c17-error {{type name does not allow storage class to be specified}} \
+   c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+  union {
+char c;
+auto smth;  // c2x-error {{'auto' not allowed in union member}} \
+   c17-error {{type name does not allow storage class to be specified}} \
+   c17-error {{type 

[PATCH] D147916: [10/11][POC][Clang][RISCV] Define vget for tuple type

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:2723
+  unsigned MaxIndex = 
cast(VTupleOperand->getType())->getNumElements();
+  Index = std::min(Index, MaxIndex);
+

craig.topper wrote:
> Does SemaChecking.cpp already guarantee Index is less than Maxindex?
Actually it looks like maybe SemaChecking considers MaxIndex to be LMUL*NF? 
Does that mean vget can extract an LMUL1 value from an LMUL4 tuple?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147916

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

[PATCH] D147917: [11/11][POC][Clang][RISCV] Define vset for tuple type

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:2763
+  unsigned MaxIndex = 
cast(VTupleOperand->getType())->getNumElements();
+  Index = std::min(Index, MaxIndex);
+

Does SemaChecking already guarantee this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147917

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

[PATCH] D147916: [10/11][POC][Clang][RISCV] Define vget for tuple type

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:2723
+  unsigned MaxIndex = 
cast(VTupleOperand->getType())->getNumElements();
+  Index = std::min(Index, MaxIndex);
+

Does SemaChecking.cpp already guarantee Index is less than Maxindex?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147916

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

[PATCH] D147914: [8/11][POC][Clang][RISCV] Define tuple type variant of vloxseg2ei32 vluxseg2ei32

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:2014
+
+  llvm::Value *MaskOperand = IsMasked ? Ops[0] : nullptr;
+  llvm::Value *PtrOperand = IsMasked ? Ops[1] : Ops[0];

Same comment as last review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147914

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

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs and fields

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:2118
+  if (Packed)
+UnadjustedAlignment = std::max(UnadjustedAlignment, UnpackedFieldAlign);
   UpdateAlignment(FieldAlign, UnpackedFieldAlign, PreferredAlign);

rjmccall wrote:
> I've always felt the data flow in this function was excessively convoluted.  
> Let's puzzle it out to figure out what's going on.  Ignoring the AIX stuff 
> which I assume can't coincide with AArch64, we've got:
> 
> ```
> UnpackedFieldAlign = min(max(TyAlign, MaxAlignmentInChars), MaxFieldAlignment)
> PackedFieldAlign = min(max(1, MaxAlignmentInChars), MaxFieldAlignment)
> FieldAlign = FieldPacked ? PackedFieldAlign : UnpackedFieldAlign
> ```
> 
> where `MaxAlignmentInChars` is the highest value of all the alignment 
> attributes on the field and `MaxFieldAlignment` is the value of `#pragma 
> pack` that was active at the time of the struct definition.
> 
> Note that this gives us `PackedFieldAlign <= FieldAlign <= 
> UnpackedFieldAlign`.
> 
> So:
> 1. I think it's wrong to be checking `Packed` instead of `FieldPacked` here.  
> But:
> 2. If `FieldPacked`, then because `UnpackedFieldAlign >= FieldAlign`, the net 
> effect of these three lines is `UnadjustedAlignment = 
> std::max(UnadjustedAlignment, UnpackedFieldAlign)`.
> 3. If `!FieldPacked`, then `UnpackedFieldAlign == FieldAlign`, so the net 
> effect of these three lines is *also* `UnadjustedAlignment = 
> std::max(UnadjustedAlignment, UnpackedFieldAlign)`.
> 4. So actually you don't need to check `FieldPacked` at all; you should 
> remove the old line and just do your new one unconditionally.
> 
> Also, AAPCS64 seems to define UnadjustedAlignment as the "natural alignment", 
> and there's a doc comment saying it's the max of the type alignments.  That 
> makes me wonder if we should really be considering either the `aligned` 
> attribute or `#pragma pack` in this computation at all; maybe we should just 
> be looking at the type alignment.
I think I had a go at this over here & failed, might have some relevant notes: 
https://reviews.llvm.org/D118511#inline-1140212

But, yeah, would love to see it simplified, if possible - just the data point 
that I tried and failed recently :/ (& contributed to some of the current 
complexity)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146242

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

[PATCH] D147913: [7/11][POC][Clang][RISCV] Define tuple type variant of vssseg2e32

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:1962
+  // Intrinsic: (val0, val1, ..., ptr, stride, vl)
+  llvm::Value *MaskOperand = IsMasked ? Ops[0] : nullptr;
+  llvm::Value *PtrOperand = IsMasked ? Ops[1] : Ops[0];

Same comment as last review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147913

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

[PATCH] D147912: [6/11][POC][Clang][RISCV] Define tuple type variant of vlsseg2e32

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:1914
+
+  llvm::Value *MaskOperand = IsMasked ? Ops[0] : nullptr;
+  llvm::Value *PtrOperand = IsMasked ? Ops[1] : Ops[0];

similar comment to what I left on the last review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147912

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

[PATCH] D147911: [5/11][POC][Clang][RISCV] Define tuple type variant of vlseg2e32ff

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:1852
+  llvm::Value *PtrOperand = IsMasked ? Ops[1] : Ops[0];
+  unsigned NewVLOperandIdx = IsMasked ? 2 : 1;
+  llvm::Value *NewVLOperand = IsMasked ? Ops[2] : Ops[1];

unsigned Offset = IsMasked ? 1 : 0;
llvm::Value *MaskOperand = IsMasked ? Ops[0] : nullptr;
llvm::Value *PtrOperand = Ops[Offset];
unsigned NewVLOperandIdx = Offset + 1;
llvm::Value *NewVLOperand = Ops[Offset + 1];
...



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147911

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

[PATCH] D147731: [3/11][POC][Clang][RISCV] Add typedef of the tuple type and define tuple type variant of vlseg2e32

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:1486
+  QualType getScalableVectorTupleType(QualType EltTy, unsigned NumElts,
+  unsigned Tuple) const;
+

Use `NF` or `NumFields` instead of `Tuple`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147731

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

[PATCH] D148439: [clang-rename] Exit gracefully when no input provided

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Yeah, this should be a right approach! Thanks!




Comment at: clang/test/clang-rename/NonExistFile.cpp:1
+// RUN: not clang-rename -offset=0 -new-name=plop asdasd 2>&1 | FileCheck %s
+// CHECK: clang-rename: input file does not exist.

Rather than `asdasd` please use a meaningful name (e.g. `non-existing-file`).



Comment at: clang/tools/clang-rename/ClangRename.cpp:233
+  if (!Entry) {
+errs() << "clang-rename: input file does not exist.\n";
+return 1;

It is worth including the filename in the error message, otherwise it won't be 
possible to understand which one is missing (there can be multiple IIRC, 
right?).

Also, it's better to put this check to the top of main, where the `OP` if first 
declared, since this is a sanity check and we want to fail if the inputs are 
corrupted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148439

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

[PATCH] D146873: [2/11][POC][Clang][RISCV] Define RVV tuple types

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:632
+#define RVV_TYPE(Name, Id, SingletonId)
\
+  case BuiltinType::Id: {  
\
+ASTContext::BuiltinVectorTypeInfo Info =   
\

Why did this code need to move into the macro body? It was written the way it 
was before to avoid code duplication.



Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:641
+  case BuiltinType::Id: {  
\
+ASTContext::BuiltinVectorTypeInfo Info =   
\
+Context.getBuiltinVectorTypeInfo(cast(Ty));   
\

Can you unify tuple and non-tuple by checking Info.NumVectors != 1?



Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:645
+ConvertType(Info.ElementType), Info.EC.getKnownMinValue());
\
+llvm::SmallVector EltTys; 
\
+for (unsigned I = 0; I < Info.NumVectors; ++I) 
\

`llvm::SmallVector EltTys(Info.NumVectors, EltTy)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146873

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

[PATCH] D143364: [RISCV] Support scalar/fix-length vector NTLH intrinsic with different domain

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143364

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

[PATCH] D148671: [Driver] Make -fsanitize=kcfi,function incompatible

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: nickdesaulniers, samitolvanen, peter.smith.
Herald added subscribers: yaneury, supersymetrie, Chia-hungDuan, cryptoad.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

A -fsanitize=kcfi instrumented function has a special instruction/data
before the function entry at a fixed offset.
A -fsanitize=function instrumented function will do a similar thing
after D148665 .

The two instrumentations are therefore incompatible.

(-kcfi doesn't need RequiresPIE. I am going to push this change
separately if it looks good.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148671

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/fsanitize.c


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -674,6 +674,9 @@
 // RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi 
-fno-sanitize-recover=kcfi %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-KCFI-RECOVER
 // CHECK-KCFI-RECOVER: error: unsupported argument 'kcfi' to option 
'-fno-sanitize-recover='
 
+// RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi,function %s -### 2>&1 
| FileCheck %s --check-prefix=CHECK-KCFI-FUNCTION
+// CHECK-KCFI-FUNCTION: error: invalid argument '-fsanitize=kcfi' not allowed 
with '-fsanitize=function'
+
 // RUN: %clang_cl -fsanitize=address -c -MDd -### -- %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-ASAN-DEBUGRTL
 // RUN: %clang_cl -fsanitize=address -c -MTd -### -- %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-ASAN-DEBUGRTL
 // RUN: %clang_cl -fsanitize=address -c -LDd -### -- %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-ASAN-DEBUGRTL
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -39,7 +39,7 @@
 static const SanitizerMask NotAllowedWithMinimalRuntime =
 SanitizerKind::Function | SanitizerKind::Vptr;
 static const SanitizerMask RequiresPIE =
-SanitizerKind::DataFlow | SanitizerKind::Scudo | SanitizerKind::KCFI;
+SanitizerKind::DataFlow | SanitizerKind::Scudo;
 static const SanitizerMask NeedsUnwindTables =
 SanitizerKind::Address | SanitizerKind::HWAddress | SanitizerKind::Thread |
 SanitizerKind::Memory | SanitizerKind::DataFlow;
@@ -517,7 +517,8 @@
   std::make_pair(SanitizerKind::MemTag,
  SanitizerKind::Address | SanitizerKind::KernelAddress |
  SanitizerKind::HWAddress |
- SanitizerKind::KernelHWAddress)};
+ SanitizerKind::KernelHWAddress),
+  std::make_pair(SanitizerKind::KCFI, SanitizerKind::Function)};
   // Enable toolchain specific default sanitizers if not explicitly disabled.
   SanitizerMask Default = TC.getDefaultSanitizers() & ~AllRemove;
 


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -674,6 +674,9 @@
 // RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi -fno-sanitize-recover=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-RECOVER
 // CHECK-KCFI-RECOVER: error: unsupported argument 'kcfi' to option '-fno-sanitize-recover='
 
+// RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi,function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-FUNCTION
+// CHECK-KCFI-FUNCTION: error: invalid argument '-fsanitize=kcfi' not allowed with '-fsanitize=function'
+
 // RUN: %clang_cl -fsanitize=address -c -MDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
 // RUN: %clang_cl -fsanitize=address -c -MTd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
 // RUN: %clang_cl -fsanitize=address -c -LDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -39,7 +39,7 @@
 static const SanitizerMask NotAllowedWithMinimalRuntime =
 SanitizerKind::Function | SanitizerKind::Vptr;
 static const SanitizerMask RequiresPIE =
-SanitizerKind::DataFlow | SanitizerKind::Scudo | SanitizerKind::KCFI;
+SanitizerKind::DataFlow | SanitizerKind::Scudo;
 static const SanitizerMask NeedsUnwindTables =
 SanitizerKind::Address | SanitizerKind::HWAddress | SanitizerKind::Thread |
 SanitizerKind::Memory | SanitizerKind::DataFlow;
@@ -517,7 +517,8 @@
   std::make_pair(SanitizerKind::MemTag,
  SanitizerKind::Address | SanitizerKind::KernelAddress |
  SanitizerKind::HWAddress |
- 

[PATCH] D148573: Port -fsanitize=function to AArch64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 514768.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Make -mbranch-protection=bti -fsanitize=function work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148573

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/CodeGen/ubsan-function.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/test/CodeGen/AArch64/func-sanitizer.ll
  llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll

Index: llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
===
--- llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
+++ llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
@@ -1,5 +1,8 @@
 ; RUN: llc -mtriple=aarch64 %s -o - | FileCheck %s
 
+@_ZTIFvvE = linkonce_odr constant i32 2
+@__llvm_rtti_proxy = private unnamed_addr constant ptr @_ZTIFvvE
+
 define void @f0() "patchable-function-entry"="0" "branch-target-enforcement"="true" {
 ; CHECK-LABEL: f0:
 ; CHECK-NEXT: .Lfunc_begin0:
@@ -85,3 +88,22 @@
   call void asm sideeffect "", ""()
   ret void
 }
+
+;; Test the interaction with -fsanitize=function.
+; CHECK:  .type sanitize_function,@function
+; CHECK-NEXT: .Ltmp{{.*}}:
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   .word   335544322  // 0x1402
+; CHECK-NEXT:   .word   .L__llvm_rtti_proxy-sanitize_function
+; CHECK-NEXT: sanitize_function:
+; CHECK-NEXT: .Lfunc_begin{{.*}}:
+; CHECK-NEXT:   .cfi_startproc
+; CHECK-NEXT:   // %bb.0:
+; CHECK-NEXT:   hint #34
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   ret
+define void @sanitize_function(ptr noundef %x) "patchable-function-prefix"="1" "patchable-function-entry"="1" "branch-target-enforcement"="true" !func_sanitize !0 {
+  ret void
+}
+
+!0 = !{i32 335544322, ptr @__llvm_rtti_proxy}
Index: llvm/test/CodeGen/AArch64/func-sanitizer.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/func-sanitizer.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck %s
+
+; CHECK-LABEL: .type _Z3funv,@function
+; CHECK-NEXT:.word   335544322  // 0x1402
+; CHECK-NEXT:.word   .L__llvm_rtti_proxy-_Z3funv
+; CHECK-NEXT:  _Z3funv:
+; CHECK-NEXT:  // %bb.0:
+; CHECK-NEXT:ret
+
+; CHECK:   .section .rodata,"a",@progbits
+; CHECK-LABEL: .L__llvm_rtti_proxy:
+; CHECK-NEXT:.xword  _ZTIFvvE
+; CHECK-NEXT:.size   .L__llvm_rtti_proxy, 8
+
+@_ZTIFvvE = linkonce_odr constant i32 1
+@__llvm_rtti_proxy = private unnamed_addr constant ptr @_ZTIFvvE
+
+define dso_local void @_Z3funv() nounwind !func_sanitize !0 {
+  ret void
+}
+
+!0 = !{i32 335544322, ptr @__llvm_rtti_proxy}
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -972,7 +972,7 @@
 
   // Emit the function prologue data for the indirect call sanitizer.
   if (const MDNode *MD = F.getMetadata(LLVMContext::MD_func_sanitize)) {
-assert(TM.getTargetTriple().isX86());
+assert(TM.getTargetTriple().isAArch64() || TM.getTargetTriple().isX86());
 assert(MD->getNumOperands() == 2);
 
 auto *PrologueSig = mdconst::extract(MD->getOperand(0));
Index: compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
===
--- compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
@@ -1,3 +1,3 @@
-# The function type checker is only supported on x86 and x86_64 for now.
-if config.target_arch not in ['x86', 'x86_64']:
+# The function type checker is only supported on aarch64 and x86 for now.
+if config.target_arch not in ['aarch64', 'x86', 'x86_64']:
   config.unsupported = True
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -525,9 +525,10 @@
 // RUN: %clang --target=x86_64-linux-gnu -fsanitize=thread -fsanitize-thread-atomics -fno-sanitize-thread-atomics %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-ATOMICS-BOTH-OFF
 // CHECK-TSAN-ATOMICS-BOTH-OFF: -cc1{{.*}}tsan-instrument-atomics=0
 
-// RUN: %clang --target=x86_64-apple-darwin10 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FSAN-DARWIN
-// RUN: %clang --target=i386-apple-darwin10 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FSAN-DARWIN
-// CHECK-FSAN-DARWIN: -cc1{{.*}}"-fsanitize=function" "-fsanitize-recover=function"
+// 

[PATCH] D148665: Change -fsanitize=function to place two words before the function entry

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 514761.
MaskRay added a comment.

fix clang/test/CodeGen


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148665

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGen/ubsan-function.cpp
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/test/CodeGen/X86/func-sanitizer.ll
  llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll

Index: llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll
===
--- llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll
+++ llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll
@@ -1,6 +1,9 @@
 ; RUN: llc -mtriple=i686 %s -o - | FileCheck --check-prefixes=CHECK,32 %s
 ; RUN: llc -mtriple=x86_64 %s -o - | FileCheck --check-prefixes=CHECK,64 %s
 
+@_ZTIFvvE = linkonce_odr constant i32 1
+@__llvm_rtti_proxy = private unnamed_addr constant ptr @_ZTIFvvE
+
 ;; -fpatchable-function-entry=0 -fcf-protection=branch
 define void @f0() "patchable-function-entry"="0" {
 ; CHECK-LABEL: f0:
@@ -83,6 +86,25 @@
   ret void
 }
 
+;; Test the interaction with -fsanitize=function.
+; CHECK:  .type sanitize_function,@function
+; CHECK-NEXT: .Ltmp{{.*}}:
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   .long   846595819
+; CHECK-NEXT:   .long   .L__llvm_rtti_proxy-sanitize_function
+; CHECK-NEXT: sanitize_function:
+; CHECK-NEXT: .Lfunc_begin{{.*}}:
+; CHECK-NEXT:   .cfi_startproc
+; CHECK-NEXT:   # %bb.0:
+; 32-NEXT:  endbr32
+; 64-NEXT:  endbr64
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   ret
+define void @sanitize_function(ptr noundef %x) "patchable-function-prefix"="1" "patchable-function-entry"="1" !func_sanitize !1 {
+  ret void
+}
+
 !llvm.module.flags = !{!0}
 
 !0 = !{i32 8, !"cf-protection-branch", i32 1}
+!1 = !{i32 846595819, ptr @__llvm_rtti_proxy}
Index: llvm/test/CodeGen/X86/func-sanitizer.ll
===
--- llvm/test/CodeGen/X86/func-sanitizer.ll
+++ llvm/test/CodeGen/X86/func-sanitizer.ll
@@ -1,12 +1,12 @@
 ; RUN: llc -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
 
-; CHECK: _Z3funv:
-; CHECK: .cfi_startproc
-; CHECK: .long   846595819
-; CHECK: .long   .L__llvm_rtti_proxy-_Z3funv
-; CHECK: .L__llvm_rtti_proxy:
-; CHECK: .quad   i
-; CHECK: .size   .L__llvm_rtti_proxy, 8
+; CHECK:  .type _Z3funv,@function
+; CHECK-NEXT:   .long   846595819  # 0x327606eb
+; CHECK-NEXT:   .long   .L__llvm_rtti_proxy-_Z3funv
+; CHECK-NEXT: _Z3funv:
+; CHECK-NEXT:   .cfi_startproc
+; CHECK-NEXT:   # %bb.0:
+; CHECK-NEXT:   retq
 
 @i = linkonce_odr constant i32 1
 @__llvm_rtti_proxy = private unnamed_addr constant ptr @i
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -970,6 +970,22 @@
 CurrentPatchableFunctionEntrySym = CurrentFnBegin;
   }
 
+  // Emit the function prologue data for the indirect call sanitizer.
+  if (const MDNode *MD = F.getMetadata(LLVMContext::MD_func_sanitize)) {
+assert(TM.getTargetTriple().isX86());
+assert(MD->getNumOperands() == 2);
+
+auto *PrologueSig = mdconst::extract(MD->getOperand(0));
+auto *FTRTTIProxy = mdconst::extract(MD->getOperand(1));
+emitGlobalConstant(F.getParent()->getDataLayout(), PrologueSig);
+
+const MCExpr *Proxy = lowerConstant(FTRTTIProxy);
+const MCExpr *FnExp = MCSymbolRefExpr::create(CurrentFnSym, OutContext);
+const MCExpr *PCRel = MCBinaryExpr::createSub(Proxy, FnExp, OutContext);
+// Use 32 bit since only small code model is supported.
+OutStreamer->emitValue(PCRel, 4u);
+  }
+
   if (isVerbose()) {
 F.printAsOperand(OutStreamer->getCommentOS(),
  /*PrintType=*/false, F.getParent());
@@ -1024,24 +1040,6 @@
   // Emit the prologue data.
   if (F.hasPrologueData())
 emitGlobalConstant(F.getParent()->getDataLayout(), F.getPrologueData());
-
-  // Emit the function prologue data for the indirect call sanitizer.
-  if (const MDNode *MD = F.getMetadata(LLVMContext::MD_func_sanitize)) {
-assert(TM.getTargetTriple().getArch() == Triple::x86 ||
-   TM.getTargetTriple().getArch() == Triple::x86_64);
-assert(MD->getNumOperands() == 2);
-
-auto *PrologueSig = mdconst::extract(MD->getOperand(0));
-auto *FTRTTIProxy = mdconst::extract(MD->getOperand(1));
-assert(PrologueSig && FTRTTIProxy);
-emitGlobalConstant(F.getParent()->getDataLayout(), PrologueSig);
-
-const MCExpr *Proxy = lowerConstant(FTRTTIProxy);
-const MCExpr *FnExp = MCSymbolRefExpr::create(CurrentFnSym, OutContext);
-const MCExpr *PCRel = MCBinaryExpr::createSub(Proxy, FnExp, OutContext);
-// Use 32 bit since only small code model is supported.
-

[PATCH] D148573: Port -fsanitize=function to AArch64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D148573#4277573 , @peter.smith 
wrote:

> As it stands I think this may have problems with -mbranch-protection. In that 
> case we'll need a `BTI c` to be the target of the indirect branch. I'm 
> guessing something like:
>
>   _Z3funv
>   BTI C ; In hint space
>   B . + 8 
>   .word .L__llvm_rtti_proxy-_Z3funv
>
> Otherwise when the indirect call is made then it will fail on a system with 
> BTI enabled.
>
> Not too sure how much of a problem this is for the implementation. The `BTI 
> c` can't be used as a signature, I guess the code in the caller could check 
> the value at `_z3funv + 4` . The feature could be marked as incompatible with 
> `-mbranch-protection`. I guess it may not work well with patchable functions 
> either.
>
> I would expect the emitGlobalConstant to emit data. This would be visible in 
> the object file as we'd have:
>
>   $d
>   
>   
>   $x
>   instructions.
>
> At the moment I don't think that this would affect anything except 
> disassemblers, and the LLD cortex-a53 eratta work around which excludes $d 
> from the disassembly. It is something that it could be worth fixing, 
> expecially if there is a `BTI C` involved.

Thanks. I forgot to check this interaction with `-mbranch-protection=bti`. x86 
`-fcf-protection=branch` has a similar problem.

The current AArch64 bti instrumentation always adds a bti (even for local 
linkage functions that are not taken addresses) to accommodate range extension 
thunks, but I can think of possible improvement to make `bti` optional in more 
cases in the future.

I think `-fsanitize=function` has to do something similar to `-fsanitize=kcfi` 
by moving the two words before the function entry: D148665 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148573

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

[PATCH] D148665: Change -fsanitize=function to place two words before the function entry

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: dmgreen, lenary, pcc, peter.smith.
Herald added subscribers: Enna1, hiraditya, kristof.beyls.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added projects: clang, Sanitizers, LLVM.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits.

The current implementation of -fsanitize=function places two words (the prolog
signature and the RTTI proxy) at the function entry, which makes the feature
incompatible with Intel Indirect Branch Tracking that needs an ENDBR instruction
at the function entry. To allow the combination, move the two words before the
function entry, similar to -fsanitize=kcfi.

The code will also be shared with my pending patch implementing
-fsanitize=function for AArch64 (Branch Target Identification has a similar
requirement).

---

For the removed function in AsmPrinter.cpp, remove an assert: `mdconst::extract`
already asserts non-nullness.

For compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp,
when the function doesn't have prolog/epilog (-O1 and above), after moving the 
two words,
the address of the function equals the address of ret instruction,
so symbolizing the function will additionally get a non-zero column number.
Adjust the test to allow an optional column number.

.long   846595819
.long   .L__llvm_rtti_proxy-_Z1fv
  _Z1fv:   // symbolizing here retrieves the line table entry from the second 
.loc
.file   0 ...
.loc0 1 0
.cfi_startproc
.loc0 2 1 prologue_end
retq


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148665

Files:
  clang/lib/CodeGen/CGExpr.cpp
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/test/CodeGen/X86/func-sanitizer.ll
  llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll

Index: llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll
===
--- llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll
+++ llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll
@@ -1,6 +1,9 @@
 ; RUN: llc -mtriple=i686 %s -o - | FileCheck --check-prefixes=CHECK,32 %s
 ; RUN: llc -mtriple=x86_64 %s -o - | FileCheck --check-prefixes=CHECK,64 %s
 
+@_ZTIFvvE = linkonce_odr constant i32 1
+@__llvm_rtti_proxy = private unnamed_addr constant ptr @_ZTIFvvE
+
 ;; -fpatchable-function-entry=0 -fcf-protection=branch
 define void @f0() "patchable-function-entry"="0" {
 ; CHECK-LABEL: f0:
@@ -83,6 +86,25 @@
   ret void
 }
 
+;; Test the interaction with -fsanitize=function.
+; CHECK:  .type sanitize_function,@function
+; CHECK-NEXT: .Ltmp{{.*}}:
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   .long   846595819
+; CHECK-NEXT:   .long   .L__llvm_rtti_proxy-sanitize_function
+; CHECK-NEXT: sanitize_function:
+; CHECK-NEXT: .Lfunc_begin{{.*}}:
+; CHECK-NEXT:   .cfi_startproc
+; CHECK-NEXT:   # %bb.0:
+; 32-NEXT:  endbr32
+; 64-NEXT:  endbr64
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   ret
+define void @sanitize_function(ptr noundef %x) "patchable-function-prefix"="1" "patchable-function-entry"="1" !func_sanitize !1 {
+  ret void
+}
+
 !llvm.module.flags = !{!0}
 
 !0 = !{i32 8, !"cf-protection-branch", i32 1}
+!1 = !{i32 846595819, ptr @__llvm_rtti_proxy}
Index: llvm/test/CodeGen/X86/func-sanitizer.ll
===
--- llvm/test/CodeGen/X86/func-sanitizer.ll
+++ llvm/test/CodeGen/X86/func-sanitizer.ll
@@ -1,12 +1,12 @@
 ; RUN: llc -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
 
-; CHECK: _Z3funv:
-; CHECK: .cfi_startproc
-; CHECK: .long   846595819
-; CHECK: .long   .L__llvm_rtti_proxy-_Z3funv
-; CHECK: .L__llvm_rtti_proxy:
-; CHECK: .quad   i
-; CHECK: .size   .L__llvm_rtti_proxy, 8
+; CHECK:  .type _Z3funv,@function
+; CHECK-NEXT:   .long   846595819  # 0x327606eb
+; CHECK-NEXT:   .long   .L__llvm_rtti_proxy-_Z3funv
+; CHECK-NEXT: _Z3funv:
+; CHECK-NEXT:   .cfi_startproc
+; CHECK-NEXT:   # %bb.0:
+; CHECK-NEXT:   retq
 
 @i = linkonce_odr constant i32 1
 @__llvm_rtti_proxy = private unnamed_addr constant ptr @i
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -970,6 +970,22 @@
 CurrentPatchableFunctionEntrySym = CurrentFnBegin;
   }
 
+  // Emit the function prologue data for the indirect call sanitizer.
+  if (const MDNode *MD = F.getMetadata(LLVMContext::MD_func_sanitize)) {
+assert(TM.getTargetTriple().isX86());
+assert(MD->getNumOperands() == 2);
+
+auto *PrologueSig = mdconst::extract(MD->getOperand(0));
+auto *FTRTTIProxy = mdconst::extract(MD->getOperand(1));
+emitGlobalConstant(F.getParent()->getDataLayout(), PrologueSig);
+
+const MCExpr *Proxy = lowerConstant(FTRTTIProxy);
+const MCExpr *FnExp = 

[PATCH] D148658: [clang] Make access to submodules via `iterator_range`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx updated this revision to Diff 514749.

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

https://reviews.llvm.org/D148658

Files:
  clang/include/clang/Basic/Module.h
  clang/lib/Basic/Module.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1903,18 +1903,16 @@
 SavedStrings.push_back(FilenameDup.data());
 
 HeaderFileInfoTrait::key_type Key = {
-  FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0
-};
+FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0};
 HeaderFileInfoTrait::data_type Data = {
-  Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}
-};
+Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
 // FIXME: Deal with cases where there are multiple unresolved header
 // directives in different submodules for the same header.
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
   }
-
-  Worklist.append(M->submodule_begin(), M->submodule_end());
+  auto SubmodulesRange = M->submodules();
+  Worklist.append(SubmodulesRange.begin(), SubmodulesRange.end());
 }
   }
 
@@ -2701,9 +2699,8 @@
 /// given module).
 static unsigned getNumberOfModules(Module *Mod) {
   unsigned ChildModules = 0;
-  for (auto Sub = Mod->submodule_begin(), SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub)
-ChildModules += getNumberOfModules(*Sub);
+  for (auto *Submodule : Mod->submodules())
+ChildModules += getNumberOfModules(Submodule);
 
   return ChildModules + 1;
 }
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4309,15 +4309,12 @@
 /*IsInclusionDirective=*/false);
 // Enumerate submodules.
 if (Mod) {
-  for (Module::submodule_iterator Sub = Mod->submodule_begin(),
-  SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub) {
-
+  for (auto *Submodule : Mod->submodules()) {
 Builder.AddTypedTextChunk(
-Builder.getAllocator().CopyString((*Sub)->Name));
+Builder.getAllocator().CopyString(Submodule->Name));
 Results.AddResult(Result(
 Builder.TakeString(), CCP_Declaration, CXCursor_ModuleImportDecl,
-(*Sub)->isAvailable() ? CXAvailability_Available
+Submodule->isAvailable() ? CXAvailability_Available
   : CXAvailability_NotAvailable));
   }
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1240,7 +1240,8 @@
 ModMap.resolveConflicts(Mod, /*Complain=*/false);
 
 // Queue the submodules, so their exports will also be resolved.
-Stack.append(Mod->submodule_begin(), Mod->submodule_end());
+auto SubmodulesRange = Mod->submodules();
+Stack.append(SubmodulesRange.begin(), SubmodulesRange.end());
   }
 }
 
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -429,11 +429,9 @@
   }
 
   // Recurse into submodules.
-  for (clang::Module::submodule_iterator Sub = Module->submodule_begin(),
-  SubEnd = Module->submodule_end();
-   Sub != SubEnd; ++Sub)
+  for (auto *Submodule : Module->submodules())
 if (std::error_code Err = collectModuleHeaderIncludes(
-LangOpts, FileMgr, Diag, ModMap, *Sub, Includes))
+LangOpts, FileMgr, Diag, ModMap, Submodule, Includes))
   return Err;
 
   return std::error_code();
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -605,8 +605,9 @@
   Module *Current = Stack.pop_back_val();
   if (Current->IsUnimportable) continue;
   Current->IsAvailable = true;
-  Stack.insert(Stack.end(),
-   Current->submodule_begin(), Current->submodule_end());
+  auto SubmodulesRange = Current->submodules();
+  Stack.insert(Stack.end(), SubmodulesRange.begin(),
+   SubmodulesRange.end());
 }
   }
 }
Index: clang/lib/CodeGen/CodeGenModule.cpp

[PATCH] D148663: [RFC][clangd] Use interpolation for CDB pushed via LSP protocol

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin created this revision.
DmitryPolukhin added reviewers: kadircet, nridge, sammccall, ilya-biryukov.
DmitryPolukhin added a project: clang-tools-extra.
Herald added a subscriber: arphaman.
Herald added a project: All.
DmitryPolukhin requested review of this revision.
Herald added a subscriber: MaskRay.

Now clangd only interpolates CDBs loaded from disk and doesn't make any
interpolation for CDBs pushed via LSP protocol. This diff add the same
extrapolation logic as for loaded from disk.

Test Plan: check-clangd


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148663

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -95,10 +95,14 @@
   CDB.setCompileCommand(testPath("foo.cc"), Override);
   EXPECT_THAT(CDB.getCompileCommand(testPath("foo.cc"))->CommandLine,
   Contains("-DA=3"));
-  EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), std::nullopt);
-  CDB.setCompileCommand(testPath("missing.cc"), Override);
+  // Expect interpolation from foo.cc
   EXPECT_THAT(CDB.getCompileCommand(testPath("missing.cc"))->CommandLine,
   Contains("-DA=3"));
+  // Check that explicit override replaces interpolation
+  Override = cmd(testPath("missing.cc"), "-DA=4");
+  CDB.setCompileCommand(testPath("missing.cc"), Override);
+  EXPECT_THAT(CDB.getCompileCommand(testPath("missing.cc"))->CommandLine,
+  Contains("-DA=4"));
 }
 
 TEST_F(OverlayCDBTest, GetFallbackCommand) {
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -17,6 +17,7 @@
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
 #include 
 #include 
 #include 
@@ -189,14 +190,19 @@
   getCompileCommand(PathRef File) const override;
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
 
-  /// Sets or clears the compilation command for a particular file.
-  void
-  setCompileCommand(PathRef File,
-std::optional CompilationCommand);
+  /// Sets compilation commands and return updated files.
+  llvm::StringSet<> setCompileCommands(
+  llvm::StringMap> Commands);
+
+  /// Legacy inefficient implementation that inserts one file at a time
+  /// that is implemented as a wrapper on top of setCompileCommands above.
+  void setCompileCommand(PathRef File,
+ std::optional Cmd);
 
 private:
   mutable std::mutex Mutex;
   llvm::StringMap Commands; /* GUARDED_BY(Mut) */
+  std::unique_ptr CDB; /* GUARDED_BY(Mut) */
   CommandMangler Mangler;
   std::vector FallbackFlags;
 };
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -729,6 +729,35 @@
   return Res->PI;
 }
 
+// Helper class that exposes CDB pushed via LSP protocol as
+// tooling::CompilationDatabase for interpolation.
+class InMemoryCompilationDatabase : public tooling::CompilationDatabase {
+public:
+  InMemoryCompilationDatabase(
+  llvm::StringMap )
+  : Commands(Commands) {}
+
+  std::vector
+  getCompileCommands(StringRef FilePath) const override {
+auto It = Commands.find(removeDots(FilePath));
+if (It != Commands.end())
+  return {It->second};
+return {};
+  }
+
+  std::vector getAllFiles() const override {
+std::vector Res;
+Res.reserve(Commands.size());
+for (const auto  : Commands.keys())
+  Res.push_back(S.str());
+return Res;
+  }
+
+private:
+  // Use reference to OverlayCDB::Commands to avoid copies.
+  llvm::StringMap 
+};
+
 OverlayCDB::OverlayCDB(const GlobalCompilationDatabase *Base,
std::vector FallbackFlags,
CommandMangler Mangler)
@@ -740,9 +769,11 @@
   std::optional Cmd;
   {
 std::lock_guard Lock(Mutex);
-auto It = Commands.find(removeDots(File));
-if (It != Commands.end())
-  Cmd = It->second;
+if (CDB) {
+  auto Candidates = CDB->getCompileCommands(File);
+  if (!Candidates.empty())
+Cmd = std::move(Candidates.front());
+}
   }
   if (!Cmd)
 Cmd = DelegatingCDB::getCompileCommand(File);
@@ -763,20 +794,36 @@
   return Cmd;
 }
 
-void 

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:781
+  /*ForConstraintInstantiation=*/true, /*SkipForSpecialization*/ false);
+  Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false);
+  std::optional ThisScope;

alexander-shaposhnikov wrote:
> rsmith wrote:
> > Just a small optimization: there's no point doing the transform if we have 
> > nothing to substitute.
> > Just a small optimization: there's no point doing the transform if we have 
> > nothing to substitute.
> 
> 
do not try to substitute empty MLTAL (suggested above by @rsmith)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:781
+  /*ForConstraintInstantiation=*/true, /*SkipForSpecialization*/ false);
+  Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false);
+  std::optional ThisScope;

rsmith wrote:
> Just a small optimization: there's no point doing the transform if we have 
> nothing to substitute.
> Just a small optimization: there's no point doing the transform if we have 
> nothing to substitute.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov updated this revision to Diff 514745.
alexander-shaposhnikov added a comment.

New version (address some comments)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

Files:
  clang/include/clang/Sema/Template.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaTemplate/concepts-out-of-line-def.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -816,3 +816,62 @@
 static_assert(Parent::TakesBinary::i == 0);
 }
 
+namespace TemplateInsideNonTemplateClass {
+template concept C = true;
+
+template auto L = [] U>() {};
+
+struct Q {
+  template U> friend constexpr auto decltype(L)::operator()() const;
+};
+} // namespace TemplateInsideNonTemplateClass
+
+namespace GH61959 {
+template 
+concept C = (sizeof(T0) >= 4);
+
+template
+struct Orig { };
+
+template
+struct Orig {
+  template requires C
+  void f() { }
+
+  template requires true
+  void f() { }
+};
+
+template  struct Mod {};
+
+template 
+struct Mod {
+  template  requires C
+  constexpr static int f() { return 1; }
+
+  template  requires C
+  constexpr static int f() { return 2; }
+};
+
+static_assert(Mod::f() == 1);
+static_assert(Mod::f() == 2);
+
+template
+struct Outer {
+  template
+  struct Inner {};
+
+  template
+  struct Inner {
+template
+void foo()  requires C && C && C{}
+template
+void foo()  requires true{}
+  };
+};
+
+void bar() {
+  Outer::Inner I;
+  I.foo();
+}
+}
Index: clang/test/SemaTemplate/concepts-out-of-line-def.cpp
===
--- clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -127,3 +127,220 @@
 static_assert(S::specialization("str") == SPECIALIZATION_REQUIRES);
 
 } // namespace multiple_template_parameter_lists
+
+static constexpr int CONSTRAINED_METHOD_1 = 1;
+static constexpr int CONSTRAINED_METHOD_2 = 2;
+
+namespace constrained_members {
+
+template 
+struct S {
+  template 
+  static constexpr int constrained_method();
+};
+
+template <>
+template 
+constexpr int S<1>::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template <>
+template 
+constexpr int S<2>::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S<1>::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S<2>::constrained_method() == CONSTRAINED_METHOD_2);
+
+
+template 
+concept ConceptT1T2 = true;
+
+template
+struct S12 {
+  template T4>
+  static constexpr int constrained_method();
+};
+
+template<>
+template T5>
+constexpr int S12::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template<>
+template T5>
+constexpr int S12::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S12::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S12::constrained_method() == CONSTRAINED_METHOD_2);
+
+} // namespace constrained members
+
+namespace constrained_members_of_nested_types {
+
+template 
+struct S {
+  struct Inner0 {
+struct Inner1 {
+  template 
+  static constexpr int constrained_method();
+};
+  };
+};
+
+template <>
+template 
+constexpr int S<1>::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template <>
+template 
+constexpr int S<2>::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S<1>::Inner0::Inner1::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S<2>::Inner0::Inner1::constrained_method() == CONSTRAINED_METHOD_2);
+
+
+template 
+concept ConceptT1T2 = true;
+
+template
+struct S12 {
+  struct Inner0 {
+struct Inner1 {
+  template T4>
+  static constexpr int constrained_method();
+};
+  };
+};
+
+template<>
+template T5>
+constexpr int S12::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template<>
+template T5>
+constexpr int S12::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S12::Inner0::Inner1::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S12::Inner0::Inner1::constrained_method() == CONSTRAINED_METHOD_2);
+
+} // namespace constrained_members_of_nested_types
+
+namespace constrained_member_sfinae {
+
+template struct S {
+  template
+  static constexpr int constrained_method() requires (sizeof(int[N * 1073741824 + 4]) == 16) {
+return CONSTRAINED_METHOD_1;
+  }
+
+  template
+  static constexpr int constrained_method() requires (sizeof(int[N]) == 16);
+};
+
+template<>
+template
+constexpr int S<4>::constrained_method() requires (sizeof(int[4]) == 16) {
+  return 

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:94
+def note_fe_backend_in : Note<"In function '%0'">;
+def note_fe_backend_inlined : Note<"\twhich inlined function '%0'">;
 

erichkeane wrote:
> This tab in the diagnostic is odd, we never do this, we just count on the 
> cascading notes to be clear.  Also, this probably needs bikeshedding for 
> diagnostic messages.
I've removed the tab, and changed this to what you've suggested below. Still 
open for bikeshedding, but marking this thread as done.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:860
+
+  Diags.Report(diag::note_fe_backend_in) << 
llvm::demangle(D.getCaller().str());
+

nickdesaulniers wrote:
> erichkeane wrote:
> > Could we instead just make `demangle` take a `string_view` here?  It takes 
> > it by const-ref, which shows that it doesn't really seem to need it to be a 
> > string, so I would imagine this would be a minor refactor (to add such an 
> > overload).
> Do you mean `StringRef` rather than `std::string_view`?  The progammer's 
> manual provides guidance on the former 
> (https://llvm.org/docs/ProgrammersManual.html#the-stringref-class) but not 
> that latter.
> 
> Otherwise is this what you had in mind?
> ```
> diff --git a/llvm/include/llvm/Demangle/Demangle.h 
> b/llvm/include/llvm/Demangle/Demangle.h
> index 6133d0b95bbf..c47222f883e9 100644
> --- a/llvm/include/llvm/Demangle/Demangle.h
> +++ b/llvm/include/llvm/Demangle/Demangle.h
> @@ -11,6 +11,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  namespace llvm {
>  /// This is a llvm local version of __cxa_demangle. Other than the name and
> @@ -68,7 +69,7 @@ char *dlangDemangle(const char *MangledName);
>  /// \param MangledName - reference to string to demangle.
>  /// \returns - the demangled string, or a copy of the input string if no
>  /// demangling occurred.
> -std::string demangle(const std::string );
> +std::string demangle(const std::string_view MangledName);
>  
>  bool nonMicrosoftDemangle(const char *MangledName, std::string );
>  
> diff --git a/llvm/lib/Demangle/Demangle.cpp b/llvm/lib/Demangle/Demangle.cpp
> index 9d128424cabf..408112b9248e 100644
> --- a/llvm/lib/Demangle/Demangle.cpp
> +++ b/llvm/lib/Demangle/Demangle.cpp
> @@ -26,9 +26,10 @@ static bool isDLangEncoding(const std::string 
> ) {
>   MangledName[1] == 'D';
>  }
>  
> -std::string llvm::demangle(const std::string ) {
> +std::string llvm::demangle(const std::string_view MangledName) {
>std::string Result;
> -  const char *S = MangledName.c_str();
> +  std::string Mangled(MangledName);
> +  const char *S = Mangled.c_str();
>  
>if (nonMicrosoftDemangle(S, Result))
>  return Result;
> @@ -43,7 +44,7 @@ std::string llvm::demangle(const std::string ) {
>  return Result;
>}
>  
> -  return MangledName;
> +  return Mangled;
>  }
>  
>  bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string 
> ) {
> ```
That yak shave is going on in https://reviews.llvm.org/D148566; in particular, 
it will require changes to libcxxabi, since llvm/Demangle is just copied from 
libcxxabi as the upstream source. As such, marking this thread done.

I'm happy to help clean up such crufty APIs, but we should not block this 
feature on that.



Comment at: clang/test/Frontend/backend-attribute-error-warning-optimize.c:12
   foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh 
no foo}}
+ // expected-note@* {{In function 'baz'}}
   if (x())

nickdesaulniers wrote:
> aaron.ballman wrote:
> > Instead of allowing this note to appear anywhere in the file, I think it's 
> > better to use "bookmark" comments. e.g.,
> > ```
> > void baz() { // #baz_defn
> > }
> > 
> > void foo() {
> >   baz(); // expected-note@#baz_defn {{whatever note text}}
> > }
> > ```
> > so that we're testing where the diagnostic is emitted. (This is especially 
> > important given that the changes are about location tracking.)
> oh, that's new (to me)! will do
It looks like the "bookmarks" don't work because the notes do not line+col 
info. They follow the warning/error diagnostic which does, on the bottom most 
call site.

The warning is supposed to be emitted on the callsite, not the definition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 514746.
nickdesaulniers marked 3 inline comments as done.
nickdesaulniers added a comment.

- rebase, invert inlining reporting chain, update diagnostic text


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Frontend/backend-attribute-error-warning-optimize.c
  clang/test/Frontend/backend-attribute-error-warning.c
  clang/test/Frontend/backend-attribute-error-warning.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/Transforms/Inline/dontcall-attributes.ll

Index: llvm/test/Transforms/Inline/dontcall-attributes.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/dontcall-attributes.ll
@@ -0,0 +1,84 @@
+; RUN: opt -S -o - -passes=inline %s \
+; RUN:  | FileCheck %s --check-prefixes=CHECK-BOTH,CHECK
+; RUN: opt -S -o - -passes=always-inline %s \
+; RUN:  | FileCheck %s --check-prefixes=CHECK-BOTH,CHECK-ALWAYS
+
+declare void @foo() "dontcall-warn"="oh no"
+declare void @fof() "dontcall-error"="oh no"
+
+define void @bar(i32 %x) {
+  %cmp = icmp eq i32 %x, 10
+  br i1 %cmp, label %if.then, label %if.end
+
+if.then:
+  call void @foo()
+  br label %if.end
+
+if.end:
+  ret void
+}
+
+define void @quux() {
+  call void @bar(i32 9)
+  ret void
+}
+
+; Test that @baz's call to @foo has metadata with inlining info.
+define void @baz() {
+; CHECK-LABEL: @baz(
+; CHECK-NEXT:call void @foo(), !inlined.from !0
+;
+  call void @bar(i32 10)
+  ret void
+}
+
+; Test that @zing's call to @foo has unique metadata from @baz's call to @foo.
+define void @zing() {
+; CHECK-LABEL: @zing(
+; CHECK-NEXT:call void @foo(), !inlined.from !1
+;
+  call void @baz()
+  ret void
+}
+
+; Same test but @fof has fn attr "dontcall-error"="..." rather than
+; "dontcall-warn"="...".
+define void @a() {
+  call void @fof()
+  ret void
+}
+define void @b() {
+; CHECK-LABEL: @b(
+; CHECK-NEXT: call void @fof(), !inlined.from !3
+  call void @a()
+  ret void
+}
+
+; Add some tests for alwaysinline.
+define void @always_callee() alwaysinline {
+  call void @fof()
+  ret void
+}
+define void @always_caller() alwaysinline {
+; CHECK-BOTH-LABEL: @always_caller(
+; CHECK-NEXT: call void @fof(), !inlined.from !4
+; CHECK-ALWAYS-NEXT: call void @fof(), !inlined.from !0
+  call void @always_callee()
+  ret void
+}
+define void @always_caller2() alwaysinline {
+; CHECK-BOTH-LABEL: @always_caller2(
+; CHECK-NEXT: call void @fof(), !inlined.from !5
+; CHECK-ALWAYS-NEXT: call void @fof(), !inlined.from !1
+  call void @always_caller()
+  ret void
+}
+
+; CHECK: !0 = !{!"bar"}
+; CHECK-NEXT: !1 = !{!2}
+; CHECK-NEXT: !2 = !{!"bar", !"baz"}
+; CHECK-NEXT: !3 = !{!"a"}
+; CHECK-NEXT: !4 = !{!"always_callee"}
+; CHECK-ALWAYS: !0 = !{!"always_callee"}
+; CHECK-ALWAYS-NEXT: !1 = !{!2}
+; CHECK-ALWAYS-NEXT: !2 = !{!"always_callee", !"always_caller"}
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -2395,6 +2395,19 @@
 // inlining, commonly when the callee is an intrinsic.
 if (MarkNoUnwind && !CI->doesNotThrow())
   CI->setDoesNotThrow();
+
+const Function *Callee = CI->getCalledFunction();
+if (Callee && (Callee->hasFnAttribute("dontcall-error") ||
+   Callee->hasFnAttribute("dontcall-warn"))) {
+  Metadata *MD = MDString::get(CI->getContext(), CalledFunc->getName());
+  if (MDNode *N = CI->getMetadata("inlined.from")) {
+TempMDTuple Temp = cast(N)->clone();
+Temp->push_back(MD);
+MD = MDNode::replaceWithUniqued(std::move(Temp));
+  }
+  MDTuple *MDT = MDNode::get(CI->getContext(), {MD});
+  CI->setMetadata("inlined.from", MDT);
+}
   }
 }
   }
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -12,6 +12,7 @@
 //===--===//
 
 #include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator_range.h"
@@ -433,8 +434,9 @@
   if (MDNode *MD = CI.getMetadata("srcloc"))
 LocCookie =
 mdconst::extract(MD->getOperand(0))->getZExtValue();
-  DiagnosticInfoDontCall D(F->getName(), A.getValueAsString(), Sev,
-   LocCookie);
+  DiagnosticInfoDontCall 

[PATCH] D148658: [clang] Make access to submodules via `iterator_range`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx updated this revision to Diff 514744.
Stoorx added a comment.

Fix unchanged usage of old functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148658

Files:
  clang/include/clang/Basic/Module.h
  clang/lib/Basic/Module.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1903,18 +1903,16 @@
 SavedStrings.push_back(FilenameDup.data());
 
 HeaderFileInfoTrait::key_type Key = {
-  FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0
-};
+FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0};
 HeaderFileInfoTrait::data_type Data = {
-  Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}
-};
+Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
 // FIXME: Deal with cases where there are multiple unresolved header
 // directives in different submodules for the same header.
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
   }
-
-  Worklist.append(M->submodule_begin(), M->submodule_end());
+  auto SubmodulesRange = M->submodules();
+  Worklist.append(SubmodulesRange.begin(), SubmodulesRange.end());
 }
   }
 
@@ -2701,9 +2699,8 @@
 /// given module).
 static unsigned getNumberOfModules(Module *Mod) {
   unsigned ChildModules = 0;
-  for (auto Sub = Mod->submodule_begin(), SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub)
-ChildModules += getNumberOfModules(*Sub);
+  for (auto *Submodule : Mod->submodules())
+ChildModules += getNumberOfModules(Submodule);
 
   return ChildModules + 1;
 }
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4309,15 +4309,12 @@
 /*IsInclusionDirective=*/false);
 // Enumerate submodules.
 if (Mod) {
-  for (Module::submodule_iterator Sub = Mod->submodule_begin(),
-  SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub) {
-
+  for (auto *Submodule : Mod->submodules()) {
 Builder.AddTypedTextChunk(
-Builder.getAllocator().CopyString((*Sub)->Name));
+Builder.getAllocator().CopyString(Submodule->Name));
 Results.AddResult(Result(
 Builder.TakeString(), CCP_Declaration, CXCursor_ModuleImportDecl,
-(*Sub)->isAvailable() ? CXAvailability_Available
+Submodule->isAvailable() ? CXAvailability_Available
   : CXAvailability_NotAvailable));
   }
 }
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -429,11 +429,9 @@
   }
 
   // Recurse into submodules.
-  for (clang::Module::submodule_iterator Sub = Module->submodule_begin(),
-  SubEnd = Module->submodule_end();
-   Sub != SubEnd; ++Sub)
+  for (auto *Submodule : Module->submodules())
 if (std::error_code Err = collectModuleHeaderIncludes(
-LangOpts, FileMgr, Diag, ModMap, *Sub, Includes))
+LangOpts, FileMgr, Diag, ModMap, Submodule, Includes))
   return Err;
 
   return std::error_code();
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -605,8 +605,9 @@
   Module *Current = Stack.pop_back_val();
   if (Current->IsUnimportable) continue;
   Current->IsAvailable = true;
-  Stack.insert(Stack.end(),
-   Current->submodule_begin(), Current->submodule_end());
+  auto SubmodulesRange = Current->submodules();
+  Stack.insert(Stack.end(), SubmodulesRange.begin(),
+   SubmodulesRange.end());
 }
   }
 }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6536,16 +6536,14 @@
 EmitTopLevelDecl(D);
 
   // Visit the submodules of this module.
-  for (clang::Module::submodule_iterator Sub = Mod->submodule_begin(),
- SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub) {
+  for (auto *Submodule 

[PATCH] D148639: [NFC][clang] Fix static analyzer concerns about AUTO_CAUSES_COPY

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

After seeing this review, I filed a support case with Synopsys with a request 
to stop reporting `AUTO_CAUSES_COPY` issues for small objects of class types, 
at least in the absence of additional evidence that a reference would be 
preferred.




Comment at: clang/lib/AST/ODRHash.cpp:597
   auto Bases = Record->bases();
-  for (auto Base : Bases) {
+  for (const auto  : Bases) {
 AddQualType(Base.getType());

This seems like a good change; `CXXBaseSpecifier` isn't particularly large, but 
this seems like a case where reference semantics are more intuitive anyway.



Comment at: clang/lib/Serialization/ASTReader.cpp:9426
   } else {
 for (auto IvarPair : DuplicateIvars) {
   Diag(IvarPair.first->getLocation(),

aaron.ballman wrote:
> Why is the above considered too expensive but this one is not?
I'm guessing that Coverity reported it as a second occurrence and those are 
easy to overlook.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:989
   std::vector Keys;
-  for (auto F : Files)
+  for (const auto  : Files)
 Keys.push_back(F.first);

aaron.ballman wrote:
> `FileID` is an `int`, not expensive to copy.
`Files` has type `llvm::DenseMap`, so the element type is 
`DenseMapPair`. `MarkedFile` contains a couple of vectors, 
so this looks like a win to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148639

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

[PATCH] D148369: [DependencyScanning] Canonicalize `CodeGenOptions.RelaxAll`

[Jan Svoboda via Phabricator via cfe-commits]

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

LGTM. Note that I think we should consider renaming the `-format` flag in the 
future. Ideally, it should reflect the fact that the scanner is doing different 
work, not just formatting the output differently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148369

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

[PATCH] D148653: [Header][doc] Add/revise MONITOR/MWAIT[X] descriptions

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n added a comment.

LGTM.

I'm not a maintainer to please wait a day or so to push so that others can 
review.




Comment at: clang/lib/Headers/pmmintrin.h:278
 ///the monitor event pending state. Data stored in the monitored address
 ///range causes the processor to exit the pending state.
 ///

interrupts too. Might as well add that if updating the comments.



Comment at: clang/lib/Headers/pmmintrin.h:291
 /// \param __hints
-///Optional hints for the monitoring state, which may vary by processor.
+///Optional hints for the monitoring state, which can vary by processor.
 static __inline__ void __DEFAULT_FN_ATTRS

out of curiosity, why "may" -> "can"?


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

https://reviews.llvm.org/D148653

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

[PATCH] D148658: [clang] Make access to submodules via `iterator_range`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx created this revision.
Stoorx added a reviewer: DavidSpickett.
Stoorx added a project: clang.
Herald added a project: All.
Stoorx requested review of this revision.

In file `clang/lib/Basic/Module.cpp` the `Module` class had `submodule_begin()` 
and `submodule_end()` functions to retrieve corresponding iterators for private 
vector of Modules. This commit removes mentioned functions, and replaces all of 
theirs usages with `submodules()` function and range-based for-loops.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148658

Files:
  clang/include/clang/Basic/Module.h
  clang/lib/Basic/Module.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1903,18 +1903,16 @@
 SavedStrings.push_back(FilenameDup.data());
 
 HeaderFileInfoTrait::key_type Key = {
-  FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0
-};
+FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0};
 HeaderFileInfoTrait::data_type Data = {
-  Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}
-};
+Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
 // FIXME: Deal with cases where there are multiple unresolved header
 // directives in different submodules for the same header.
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
   }
-
-  Worklist.append(M->submodule_begin(), M->submodule_end());
+  auto SubmodulesRange = M->submodules();
+  Worklist.append(SubmodulesRange.begin(), SubmodulesRange.end());
 }
   }
 
@@ -2701,9 +2699,8 @@
 /// given module).
 static unsigned getNumberOfModules(Module *Mod) {
   unsigned ChildModules = 0;
-  for (auto Sub = Mod->submodule_begin(), SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub)
-ChildModules += getNumberOfModules(*Sub);
+  for (auto *Submodule : Mod->submodules())
+ChildModules += getNumberOfModules(Submodule);
 
   return ChildModules + 1;
 }
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4309,15 +4309,12 @@
 /*IsInclusionDirective=*/false);
 // Enumerate submodules.
 if (Mod) {
-  for (Module::submodule_iterator Sub = Mod->submodule_begin(),
-  SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub) {
-
+  for (auto *Submodule : Mod->submodules()) {
 Builder.AddTypedTextChunk(
-Builder.getAllocator().CopyString((*Sub)->Name));
+Builder.getAllocator().CopyString(Submodule->Name));
 Results.AddResult(Result(
 Builder.TakeString(), CCP_Declaration, CXCursor_ModuleImportDecl,
-(*Sub)->isAvailable() ? CXAvailability_Available
+Submodule->isAvailable() ? CXAvailability_Available
   : CXAvailability_NotAvailable));
   }
 }
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -429,11 +429,9 @@
   }
 
   // Recurse into submodules.
-  for (clang::Module::submodule_iterator Sub = Module->submodule_begin(),
-  SubEnd = Module->submodule_end();
-   Sub != SubEnd; ++Sub)
+  for (auto *Submodule : Module->submodules())
 if (std::error_code Err = collectModuleHeaderIncludes(
-LangOpts, FileMgr, Diag, ModMap, *Sub, Includes))
+LangOpts, FileMgr, Diag, ModMap, Submodule, Includes))
   return Err;
 
   return std::error_code();
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -605,8 +605,9 @@
   Module *Current = Stack.pop_back_val();
   if (Current->IsUnimportable) continue;
   Current->IsAvailable = true;
-  Stack.insert(Stack.end(),
-   Current->submodule_begin(), Current->submodule_end());
+  auto SubmodulesRange = Current->submodules();
+  Stack.insert(Stack.end(), SubmodulesRange.begin(),
+   SubmodulesRange.end());
 }
   }
 }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ 

[PATCH] D147743: [Clang][NFC] Rename methods/vars to reflect their real usage

[Bill Wendling 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 rGb51a03e1bbe7: [Clang][NFC] Rename methods/vars to reflect 
their real usage (authored by void).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147743

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang/include/clang/AST/Expr.h
  clang/include/clang/Sema/Designator.h
  clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
  clang/lib/AST/Expr.cpp
  clang/lib/Index/IndexBody.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2857,7 +2857,7 @@
   for (const DesignatedInitExpr::Designator  :
llvm::reverse(E->designators())) {
 if (D.isFieldDesignator()) {
-  if (FieldDecl *Field = D.getField())
+  if (const FieldDecl *Field = D.getFieldDecl())
 AddMemberRef(Field, D.getFieldLoc());
   continue;
 }
Index: clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -228,16 +228,17 @@
 
   bool VisitDesignatedInitExpr(const DesignatedInitExpr *E) {
 for (const DesignatedInitExpr::Designator  : E->designators()) {
-  if (D.isFieldDesignator() && D.getField()) {
-const FieldDecl *Decl = D.getField();
-if (isInUSRSet(Decl)) {
-  auto StartLoc = D.getFieldLoc();
-  auto EndLoc = D.getFieldLoc();
-  RenameInfos.push_back({StartLoc, EndLoc,
- /*FromDecl=*/nullptr,
- /*Context=*/nullptr,
- /*Specifier=*/nullptr,
- /*IgnorePrefixQualifiers=*/true});
+  if (D.isFieldDesignator()) {
+if (const FieldDecl *Decl = D.getFieldDecl()) {
+  if (isInUSRSet(Decl)) {
+auto StartLoc = D.getFieldLoc();
+auto EndLoc = D.getFieldLoc();
+RenameInfos.push_back({StartLoc, EndLoc,
+   /*FromDecl=*/nullptr,
+   /*Context=*/nullptr,
+   /*Specifier=*/nullptr,
+   /*IgnorePrefixQualifiers=*/true});
+  }
 }
   }
 }
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1087,7 +1087,7 @@
   Record.push_back(E->usesGNUSyntax());
   for (const DesignatedInitExpr::Designator  : E->designators()) {
 if (D.isFieldDesignator()) {
-  if (FieldDecl *Field = D.getField()) {
+  if (FieldDecl *Field = D.getFieldDecl()) {
 Record.push_back(serialization::DESIG_FIELD_DECL);
 Record.AddDeclRef(Field);
   } else {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1218,7 +1218,7 @@
   SourceLocation FieldLoc = readSourceLocation();
   Designators.push_back(Designator::CreateFieldDesignator(
   Field->getIdentifier(), DotLoc, FieldLoc));
-  Designators.back().setField(Field);
+  Designators.back().setFieldDecl(Field);
   break;
 }
 
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -11663,10 +11663,10 @@
 if (D.isFieldDesignator()) {
   Desig.AddDesignator(Designator::CreateFieldDesignator(
   D.getFieldName(), D.getDotLoc(), D.getFieldLoc()));
-  if (D.getField()) {
+  if (D.getFieldDecl()) {
 FieldDecl *Field = cast_or_null(
-getDerived().TransformDecl(D.getFieldLoc(), D.getField()));
-if (Field != D.getField())
+getDerived().TransformDecl(D.getFieldLoc(), D.getFieldDecl()));
+if (Field != D.getFieldDecl())
   // Rebuild the expression when the transformed FieldDecl is
   // different to the already assigned FieldDecl.
   ExprChanged = true;
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ 

[clang-tools-extra] b51a03e - [Clang][NFC] Rename methods/vars to reflect their real usage

[Bill Wendling via cfe-commits]

Author: Bill Wendling
Date: 2023-04-18T13:48:08-07:00
New Revision: b51a03e1bbe7ea8868ffb24472fc532d0c00943c

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

LOG: [Clang][NFC] Rename methods/vars to reflect their real usage

The "getField" method is a bit confusing considering we also have a
"getFieldName" method. Instead, use "getFieldDecl" rather than
"getField".

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

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/XRefs.cpp
clang/include/clang/AST/Expr.h
clang/include/clang/Sema/Designator.h
clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
clang/lib/AST/Expr.cpp
clang/lib/Index/IndexBody.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
clang/tools/libclang/CIndex.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index 1caf70183fdb6..efbd05e2b74d3 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -288,7 +288,7 @@ struct TargetFinder {
 for (const DesignatedInitExpr::Designator  :
  llvm::reverse(DIE->designators()))
   if (D.isFieldDesignator()) {
-Outer.add(D.getField(), Flags);
+Outer.add(D.getFieldDecl(), Flags);
 // We don't know which designator was intended, we assume the 
outer.
 break;
   }
@@ -808,7 +808,7 @@ llvm::SmallVector refInStmt(const Stmt *S,
 Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
 D.getFieldLoc(),
 /*IsDecl=*/false,
-{D.getField()}});
+{D.getFieldDecl()}});
   }
 }
 

diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 9a1bd7dbd2564..23dd72d43d3bb 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -1907,7 +1907,7 @@ static QualType typeForNode(const SelectionTree::Node *N) 
{
 // In .foo.bar we want to jump to bar's type, so find *last* field.
 for (auto  : llvm::reverse(S->designators()))
   if (D.isFieldDesignator())
-if (const auto *FD = D.getField())
+if (const auto *FD = D.getFieldDecl())
   return FD->getType();
 return QualType();
   }

diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index a9941d9e8af49..a7e28c852d6ec 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -5163,7 +5163,7 @@ class DesignatedInitExpr final
 
//===--===//
 // FieldDesignatorInfo
 
-/// Initializes a field designator.
+/// Creates a field designator.
 static Designator CreateFieldDesignator(const IdentifierInfo *FieldName,
 SourceLocation DotLoc,
 SourceLocation FieldLoc) {
@@ -5174,15 +5174,14 @@ class DesignatedInitExpr final
 
 const IdentifierInfo *getFieldName() const;
 
-FieldDecl *getField() const {
+FieldDecl *getFieldDecl() const {
   assert(isFieldDesignator() && "Only valid on a field designator");
   if (FieldInfo.NameOrField & 0x01)
 return nullptr;
-  else
-return reinterpret_cast(FieldInfo.NameOrField);
+  return reinterpret_cast(FieldInfo.NameOrField);
 }
 
-void setField(FieldDecl *FD) {
+void setFieldDecl(FieldDecl *FD) {
   assert(isFieldDesignator() && "Only valid on a field designator");
   FieldInfo.NameOrField = reinterpret_cast(FD);
 }
@@ -5200,7 +5199,7 @@ class DesignatedInitExpr final
 
//===--===//
 // ArrayOrRangeDesignator
 
-/// Initializes an array designator.
+/// Creates an array designator.
 static Designator CreateArrayDesignator(unsigned Index,
 SourceLocation LBracketLoc,
 SourceLocation RBracketLoc) {
@@ -5210,7 +5209,7 @@ class DesignatedInitExpr final
   return D;
 }
 
-/// Initializes a GNU array-range designator.
+/// Creates a GNU array-range designator.
 static Designator CreateArrayRangeDesignator(unsigned Index,
  SourceLocation 

[PATCH] D147743: [Clang][NFC] Rename methods/vars to reflect their real usage

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 514730.
void marked an inline comment as done.
void added a comment.

Add comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147743

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang/include/clang/AST/Expr.h
  clang/include/clang/Sema/Designator.h
  clang/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h
  clang/lib/AST/Expr.cpp
  clang/lib/Index/IndexBody.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2857,7 +2857,7 @@
   for (const DesignatedInitExpr::Designator  :
llvm::reverse(E->designators())) {
 if (D.isFieldDesignator()) {
-  if (FieldDecl *Field = D.getField())
+  if (const FieldDecl *Field = D.getFieldDecl())
 AddMemberRef(Field, D.getFieldLoc());
   continue;
 }
Index: clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -228,16 +228,17 @@
 
   bool VisitDesignatedInitExpr(const DesignatedInitExpr *E) {
 for (const DesignatedInitExpr::Designator  : E->designators()) {
-  if (D.isFieldDesignator() && D.getField()) {
-const FieldDecl *Decl = D.getField();
-if (isInUSRSet(Decl)) {
-  auto StartLoc = D.getFieldLoc();
-  auto EndLoc = D.getFieldLoc();
-  RenameInfos.push_back({StartLoc, EndLoc,
- /*FromDecl=*/nullptr,
- /*Context=*/nullptr,
- /*Specifier=*/nullptr,
- /*IgnorePrefixQualifiers=*/true});
+  if (D.isFieldDesignator()) {
+if (const FieldDecl *Decl = D.getFieldDecl()) {
+  if (isInUSRSet(Decl)) {
+auto StartLoc = D.getFieldLoc();
+auto EndLoc = D.getFieldLoc();
+RenameInfos.push_back({StartLoc, EndLoc,
+   /*FromDecl=*/nullptr,
+   /*Context=*/nullptr,
+   /*Specifier=*/nullptr,
+   /*IgnorePrefixQualifiers=*/true});
+  }
 }
   }
 }
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1087,7 +1087,7 @@
   Record.push_back(E->usesGNUSyntax());
   for (const DesignatedInitExpr::Designator  : E->designators()) {
 if (D.isFieldDesignator()) {
-  if (FieldDecl *Field = D.getField()) {
+  if (FieldDecl *Field = D.getFieldDecl()) {
 Record.push_back(serialization::DESIG_FIELD_DECL);
 Record.AddDeclRef(Field);
   } else {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1218,7 +1218,7 @@
   SourceLocation FieldLoc = readSourceLocation();
   Designators.push_back(Designator::CreateFieldDesignator(
   Field->getIdentifier(), DotLoc, FieldLoc));
-  Designators.back().setField(Field);
+  Designators.back().setFieldDecl(Field);
   break;
 }
 
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -11663,10 +11663,10 @@
 if (D.isFieldDesignator()) {
   Desig.AddDesignator(Designator::CreateFieldDesignator(
   D.getFieldName(), D.getDotLoc(), D.getFieldLoc()));
-  if (D.getField()) {
+  if (D.getFieldDecl()) {
 FieldDecl *Field = cast_or_null(
-getDerived().TransformDecl(D.getFieldLoc(), D.getField()));
-if (Field != D.getField())
+getDerived().TransformDecl(D.getFieldLoc(), D.getFieldDecl()));
+if (Field != D.getFieldDecl())
   // Rebuild the expression when the transformed FieldDecl is
   // different to the already assigned FieldDecl.
   ExprChanged = true;
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -2370,7 +2370,7 @@
   Replacements.push_back(Designator::CreateFieldDesignator(
   

[PATCH] D147016: [PowerPC] Add function pointer alignment to DataLayout

[Thurston Dang via Phabricator via cfe-commits]

thurston added a comment.

Ah, I just noticed you had a follow-up commit 
(https://reviews.llvm.org/rG0236564771b432ebf050144eda4ef4b7424db8c3). I'm 
re-running the tests/build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147016

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

[PATCH] D147016: [PowerPC] Add function pointer alignment to DataLayout

[Thurston Dang via Phabricator via cfe-commits]

thurston added a comment.

This is causing buildbot failures e.g., 
https://lab.llvm.org/buildbot/#/builders/5/builds/33084:

   TEST 'lld :: ELF/lto/ppc.ll' FAILED 
  ...
  ld.lld: 
/b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MachineFunction.cpp:231:
 void llvm::MachineFunction::init(): Assertion 
`Target.isCompatibleDataLayout(getDataLayout()) && "Can't create a 
MachineFunction using a Module with a " "Target-incompatible DataLayout 
attached\n"' failed.
  
/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/tools/lld/test/ELF/lto/Output/ppc.ll.script:
 line 10: 1277639 Aborted (core dumped) 
/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/ld.lld 
/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/tools/lld/test/ELF/lto/Output/ppc.ll.tmp/32le.o
 -o 
/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/tools/lld/test/ELF/lto/Output/ppc.ll.tmp/32le

I reproduced the build error locally for this commit 
(8d2e9fc8553c91b1f22e82ffc2c00020f40bb0a2 
), while 
also verifying that the immediately preceding commit 
(221350965a891fc4ce33ea311428448939678112 
) did not 
have this error.

Could you please take a look at this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147016

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

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

Alright, cool, thanks! I'll land it tomorrow evening if there are no objections.


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

https://reviews.llvm.org/D140722

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

[PATCH] D147844: Emit warning when implicit cast to bool happens in an conditional operator expression when used inside an overloaded shift operator expression

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav added a comment.

@aaron.ballman am looking into this. If i change it back to
 `operation int();`
and add
`Stream& operator<<(bool)`

I get

`
-

error: 'warning' diagnostics expected but not seen:

  File /usr/home/nvellanki/explore/llvm-project/clang/test/Sema/parentheses.cpp 
Line 68: operator '?:' has lower precedence than '<<'

error: 'note' diagnostics expected but not seen:

  File /usr/home/nvellanki/explore/llvm-project/clang/test/Sema/parentheses.cpp 
Line 68: place parentheses around the '<<' expression to silence this warning
  File /usr/home/nvellanki/explore/llvm-project/clang/test/Sema/parentheses.cpp 
Line 68: place parentheses around the '?:' expression to evaluate it first

3 errors generated.

-

`

But it works correctly with std::cout

`
test.cpp:7:30: warning: operator '?:' has lower precedence than '<<'; '<<' will 
be evaluated first [-Wparentheses]

  std::cout << "Test" << a ? 1 : 0;
   ^

test.cpp:7:30: note: place parentheses around the '<<' expression to silence 
this warning

  std::cout << "Test" << a ? 1 : 0;
   ^
  (   )

test.cpp:7:30: note: place parentheses around the '?:' expression to evaluate 
it first

  std::cout << "Test" << a ? 1 : 0;
   ^
 ()

`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D148653: [Header][doc] Add/revise MONITOR/MWAIT[X] descriptions

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.
probinson added reviewers: RKSimon, pengfei, goldstein.w.n, craig.topper.
Herald added a project: All.
probinson requested review of this revision.

https://reviews.llvm.org/D148653

Files:
  clang/lib/Headers/mwaitxintrin.h
  clang/lib/Headers/pmmintrin.h


Index: clang/lib/Headers/pmmintrin.h
===
--- clang/lib/Headers/pmmintrin.h
+++ clang/lib/Headers/pmmintrin.h
@@ -253,9 +253,12 @@
 ///the processor in the monitor event pending state. Data stored in the
 ///monitored address range causes the processor to exit the pending state.
 ///
+/// The \c MONITOR instruction can be used in kernel mode, and in other modes
+/// if MSR  C001_0015h[MonMwaitUserEn]  is set.
+///
 /// \headerfile 
 ///
-/// This intrinsic corresponds to the  MONITOR  instruction.
+/// This intrinsic corresponds to the \c MONITOR instruction.
 ///
 /// \param __p
 ///The memory range to be monitored. The size of the range is determined by
@@ -270,19 +273,22 @@
   __builtin_ia32_monitor(__p, __extensions, __hints);
 }
 
-/// Used with the MONITOR instruction to wait while the processor is in
+/// Used with the \c MONITOR instruction to wait while the processor is in
 ///the monitor event pending state. Data stored in the monitored address
 ///range causes the processor to exit the pending state.
 ///
+/// The \c MWAIT instruction can be used in kernel mode, and in other modes if
+/// MSR  C001_0015h[MonMwaitUserEn]  is set.
+///
 /// \headerfile 
 ///
-/// This intrinsic corresponds to the  MWAIT  instruction.
+/// This intrinsic corresponds to the \c MWAIT instruction.
 ///
 /// \param __extensions
-///Optional extensions for the monitoring state, which may vary by
+///Optional extensions for the monitoring state, which can vary by
 ///processor.
 /// \param __hints
-///Optional hints for the monitoring state, which may vary by processor.
+///Optional hints for the monitoring state, which can vary by processor.
 static __inline__ void __DEFAULT_FN_ATTRS
 _mm_mwait(unsigned __extensions, unsigned __hints)
 {
Index: clang/lib/Headers/mwaitxintrin.h
===
--- clang/lib/Headers/mwaitxintrin.h
+++ clang/lib/Headers/mwaitxintrin.h
@@ -16,12 +16,41 @@
 
 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__,  
__target__("mwaitx")))
+
+/// Establishes a linear address memory range to be monitored and puts
+///the processor in the monitor event pending state. Data stored in the
+///monitored address range causes the processor to exit the pending state.
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c MONITORX instruction.
+///
+/// \param __p
+///The memory range to be monitored. The size of the range is determined by
+///CPUID function _0005h.
+/// \param __extensions
+///Optional extensions for the monitoring state.
+/// \param __hints
+///Optional hints for the monitoring state.
 static __inline__ void __DEFAULT_FN_ATTRS
 _mm_monitorx(void * __p, unsigned __extensions, unsigned __hints)
 {
   __builtin_ia32_monitorx(__p, __extensions, __hints);
 }
 
+/// Used with the \c MONITORX instruction to wait while the processor is in
+///the monitor event pending state. Data stored in the monitored address
+///range causes the processor to exit the pending state.
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c MWAITX instruction.
+///
+/// \param __extensions
+///Optional extensions for the monitoring state, which can vary by
+///processor.
+/// \param __hints
+///Optional hints for the monitoring state, which can vary by processor.
 static __inline__ void __DEFAULT_FN_ATTRS
 _mm_mwaitx(unsigned __extensions, unsigned __hints, unsigned __clock)
 {


Index: clang/lib/Headers/pmmintrin.h
===
--- clang/lib/Headers/pmmintrin.h
+++ clang/lib/Headers/pmmintrin.h
@@ -253,9 +253,12 @@
 ///the processor in the monitor event pending state. Data stored in the
 ///monitored address range causes the processor to exit the pending state.
 ///
+/// The \c MONITOR instruction can be used in kernel mode, and in other modes
+/// if MSR  C001_0015h[MonMwaitUserEn]  is set.
+///
 /// \headerfile 
 ///
-/// This intrinsic corresponds to the  MONITOR  instruction.
+/// This intrinsic corresponds to the \c MONITOR instruction.
 ///
 /// \param __p
 ///The memory range to be monitored. The size of the range is determined by
@@ -270,19 +273,22 @@
   __builtin_ia32_monitor(__p, __extensions, __hints);
 }
 
-/// Used with the MONITOR instruction to wait while the processor is in
+/// Used with the \c MONITOR instruction to wait while the processor is in
 ///the monitor event pending state. Data stored in the monitored address
 

[PATCH] D146595: [clang] Add "debug_trampoline" attribute

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D146595#4235340 , @augusto2112 
wrote:

> In D146595#4235333 , @aprantl wrote:
>
>> I hope I'm not kicking off a long bike-shedding thread, but I would propose 
>> to either call the attribute `transparent` to match the DWARF attribute 
>> name, or if we want to be more descriptive, `debug_transparent`, or 
>> `transparentdebug` to fit better with other attributes such as `nodebug`. My 
>> vote is on `transparent`.
>
> The dwarf attribute is actually called `trampoline`, not `transparent`. 
> `trampoline` is a pretty generic name which could be used for other things 
> other than debugging, so maybe `trampolinedebug`?

FWIW, trampoline seems weird to me (both for the DWARF attribute, and the 
source level attribute) - my understanding is that trampolines are small 
functions that just immediately jump to some other function. In this case the 
intermediate function can be arbitrarily complicated, but just not 
/interesting/ to the average developer. Doesn't seem like a trampoline to me. 
(though maybe I'm thinking of a "thunk" specifically - at least wikipedia's 
deefinition of a programming trampoline is fairly broad/involves some fairly 
higher level "do some non-trivial things" operations, I guess)

I guess an example of something that might be transparent but not 
trampoline-like would be std::sort - the ability to step into the comparison 
function, but skip over the actual sorting logic (or for_each, etc - you might 
want to step into your functor, but skip the intermediate logic that's handling 
the looping, etc). Those don't feel like trampolines, I guess?

In D146595#4241852 , @aprantl wrote:

>> Is there some place (bug, discourse thread, etc) where the broader direction 
>> is discussed? I want to checkin on the design decisions/alternatives without 
>> fragmenting this across multiple reviews/losing context/etc?
>
> No, I believe that all the relevant discussion happened in this review. While 
> there is a separate review for an LLDB implementation, there is no general 
> direction discussion there, it's just implementing a DWARF feature.

Thanks, sorry, gets all a bit confusing sometimes :/

>> (specifically - this started out with the trampoline attribute, then 
>> switched to this transparent idea (perhaps based on my feedback? Also other 
>> feedback? I'd like to know more about how that change in direction happened, 
>> what the tradeoffs were, etc - I don't think my suggestion alone was 
>> probably enough to make this direction clearly the right one (nor clearly 
>> the wrong one)), etc)
>
> It was partially based on your feedback, but also on @arphaman pointing out 
> that the `DW_AT_trampoline("call_target")` implementation wouldn't be able to 
> deal with the jump target being a virtual function call. So @augusto2112 
> landed on implementing the "flag variant" of `DW_AT_trampiline` instead. This 
> is also an existing DWARF feature, albeit not yet supported by LLVM.

OK

>> & there was some tangent about DWARF v COFF too, which I wouldn't mind 
>> weighing in on, but feel like it's all a bit fragmented, so not sure where 
>> all the discussions are/how to keep track of them.
>
> That was also in this review; @aaron.ballman pointed out that it would be 
> best if new Clang attributes weren't targeting only DWARF, though I believe 
> this request may run into some hard limitations of what CodeView/PDB can 
> support.

Yeah, that seems like a bit of a big ask, since there's a specific DWARF 
feature we're lowering to here - feels out of scope to go trying to figure out 
how it might be implemented in CodeView. For features that are format-agnostic 
(like simple template names, no-standalone-debug (various type homing 
strategies), etc) yeah, makes total sense for them to be format-agnostic. This 
isn't one of those, though.

but, I guess, worth at least asking the question if there's an equivalent 
feature in CV already that this should be mapped to


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D147928: [clang] Keep multiple-include optimization for null directives

[Elliot Goodrich via Phabricator via cfe-commits]

IncludeGuardian added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:1183-1185
+if (!ReadAnyTokensBeforeDirective) {
+  CurPPLexer->MIOpt.ResetReadToken();
+}

aaron.ballman wrote:
> NFC but matches our style guide.
> 
> One thing to consider: making the call a setter instead of a reset would 
> simplify this logic into: 
> `CurPPLexer->SetReadToken(ReadAnyTokensBeforeDirective);` without any 
> branching required. WDYT?
Sounds good. Removing the condition here should look neater.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147928

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

[PATCH] D148639: [NFC][clang] Fix static analyzer concerns about AUTO_CAUSES_COPY

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for the static analysis fixes, however, it looks like quite a few of 
them were false positives again. When fixing any static analysis diagnostics, 
please put in the effort to determine whether it's actually correct or not, as 
it seem the Coverity diagnostic is not really paying any attention to the size 
of what's being copied. If you're applying the advice from the tool without 
further analysis, it can be disruptive (might introduce bugs, churns the code 
for negative/no benefit, lowers reviewer confidence in the rest of the changes, 
etc). (Not suggesting you're blindly applying advice, but mostly observing 
we've had a few reviews like this.)

My suggestion is to always look at the size of what's being copied. As a 
general rule of thumb, if it's two pointers or less in size, the copy is likely 
cheaper than the reference and shouldn't be changed.




Comment at: clang/lib/AST/ExprConstant.cpp:6117
   } else
-for (auto Idx : Attr->args()) {
+for (const auto  : Attr->args()) {
   unsigned ASTIdx = Idx.getASTIndex();

This returns a `ParamIdx` object which is 32-bits, so it's not expensive to 
copy.



Comment at: clang/lib/Basic/TargetID.cpp:136
 OrderedMap[F.first()] = F.second;
-  for (auto F : OrderedMap)
+  for (const auto  : OrderedMap)
 TargetID = TargetID + ':' + F.first.str() + (F.second ? "+" : "-");

The map iterator returns a `std::pair` which is not 
particularly expensive to copy, but I think this is borderline enough that it's 
fine to keep.



Comment at: clang/lib/Basic/Targets/ARM.cpp:463
 
-  for (auto Feature : TargetFeatures)
+  for (const auto  : TargetFeatures)
 if (Feature[0] == '+')

This returns a `StringRef` which is not expensive to copy.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3548
   SmallVector, 5> Checks;
-  for (auto CheckKindMaskPair : CheckKinds) {
+  for (const auto  : CheckKinds) {
 int Kind = CheckKindMaskPair.first;

This is a pair of ints, not expensive to copy.



Comment at: clang/lib/Serialization/ASTReader.cpp:9426
   } else {
 for (auto IvarPair : DuplicateIvars) {
   Diag(IvarPair.first->getLocation(),

Why is the above considered too expensive but this one is not?



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:989
   std::vector Keys;
-  for (auto F : Files)
+  for (const auto  : Files)
 Keys.push_back(F.first);

`FileID` is an `int`, not expensive to copy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148639

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

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D148467#4277960 , @MyDeveloperDay 
wrote:

>>>   public Foo {
>>>   set;
>>>   get;
>>>   }
>>
>> At least from my experience, the getter is specified before the setter, 
>> though I'm unsure how important this is in your eyes.
>
> Lets hold off the idea of swapping `set;get` around from this patch, but this 
> is something that I think we could do..  (using the FormatTokenLexer.cpp to 
> swap trivial `set;get` into `get;set` but lets leave that for another 
> feature, as it would be code altering)
>
> honeslty I think we need another option  maybe
>
>   enum AllowShortCSharpProperties {
> Leave,
> Never,
> Empty,
> Always
>   }
>
> **Leave **= Don't touch them
> **Never **=  always break  (braces follow AfterCSharpProperty
>
>   set;
>   get;
>   
>   set { 
> val = value; 
>   }
>   get { 
> return = value; 
>   }
>
> **Empty**  = only one the same line when empty
>
>   set; get
>   
>   set { 
>  val = value; 
>   }
>   get { 
> return = value;
>   }
>
> **Always **=  always short form for trivial
>
>   set; get;
>   
>   set {  val = value; }
>   get {  return = value; }
>
> I'm trying to decide if I put that option in this change or in a seperate 
> change (thoughts @HazardyKnusperkeks, @owenpan )
>
> FYI, I've solved the indention issue in my branch which has been broken 
> forever from what I can tell.
>
>   verifyFormat("class A\n"
>"{\n"
>"string Bar {\n"
>"get;\n"
>"set\n"
>"{\n"
>"val = value;\n"
>"}\n"
>"}\n"
>"}",
>Style);

I'd say do it now. No need to introduce a new option, when we want to change it 
directly.

Or at least make it the enum right now, with just 2 values.


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

https://reviews.llvm.org/D148467

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

[PATCH] D147844: Emit warning when implicit cast to bool happens in an conditional operator expression when used inside an overloaded shift operator expression

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Sema/parentheses.cpp:34
 public:
-  operator int();
+  operator bool();
   Stream <<(int);

aaron.ballman wrote:
> Is this change necessary? Alternatively, can we add this as an overload 
> instead of changing the operator?
I'm still wondering about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen updated this revision to Diff 514708.
samitolvanen added a comment.

Addressed most of the feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148385

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/lib/Target/RISCV/CMakeLists.txt
  llvm/lib/Target/RISCV/RISCV.h
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
  llvm/lib/Target/RISCV/RISCVInstrInfo.h
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/lib/Target/RISCV/RISCVKCFI.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/test/CodeGen/RISCV/O0-pipeline.ll
  llvm/test/CodeGen/RISCV/O3-pipeline.ll
  llvm/test/CodeGen/RISCV/kcfi-isel.mir
  llvm/test/CodeGen/RISCV/kcfi-patchable-function-prefix.ll
  llvm/test/CodeGen/RISCV/kcfi.ll
  llvm/test/CodeGen/RISCV/kcfi.mir

Index: llvm/test/CodeGen/RISCV/kcfi.mir
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/kcfi.mir
@@ -0,0 +1,185 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 2
+# RUN: llc -mtriple=riscv64 -stop-after=riscv-kcfi -o - %s | FileCheck %s
+--- |
+  ; ModuleID = ''
+  source_filename = ""
+  target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
+  target triple = "riscv64"
+
+  define void @f1(ptr noundef %x) !kcfi_type !1 {
+call void %x() [ "kcfi"(i32 12345678) ]
+ret void
+  }
+
+  define void @f2(ptr noundef %x) #0 {
+tail call void %x() [ "kcfi"(i32 12345678) ]
+ret void
+  }
+
+  attributes #0 = { "patchable-function-entry"="2" }
+
+  !llvm.module.flags = !{!0}
+
+  !0 = !{i32 4, !"kcfi", i32 1}
+  !1 = !{i32 12345678}
+
+...
+---
+name:f1
+alignment:   4
+exposesReturnsTwice: false
+legalized:   false
+regBankSelected: false
+selected:false
+failedISel:  false
+tracksRegLiveness: true
+hasWinCFI:   false
+callsEHReturn:   false
+callsUnwindInit: false
+hasEHCatchret:   false
+hasEHScopes: false
+hasEHFunclets:   false
+isOutlined:  false
+debugInstrRef:   false
+failsVerification: false
+tracksDebugUserValues: true
+registers:   []
+liveins:
+  - { reg: '$x10', virtual-reg: '' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap: false
+  hasPatchPoint:   false
+  stackSize:   16
+  offsetAdjustment: 0
+  maxAlignment:8
+  adjustsStack:true
+  hasCalls:true
+  stackProtector:  ''
+  functionContext: ''
+  maxCallFrameSize: 0
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:  false
+  hasMustTailInVarArgFunc: false
+  hasTailCall: false
+  localFrameSize:  0
+  savePoint:   ''
+  restorePoint:''
+fixedStack:  []
+stack:
+  - { id: 0, name: '', type: spill-slot, offset: -8, size: 8, alignment: 8,
+  stack-id: default, callee-saved-register: '$x1', callee-saved-restored: true,
+  debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+callSites:   []
+debugValueSubstitutions: []
+constants:   []
+machineFunctionInfo:
+  varArgsFrameIndex: 0
+  varArgsSaveSize: 0
+body: |
+  bb.0 (%ir-block.0):
+liveins: $x10, $x1
+
+; CHECK-LABEL: name: f1
+; CHECK: liveins: $x1, $x10
+; CHECK-NEXT: {{  $}}
+; CHECK-NEXT: $x2 = frame-setup ADDI $x2, -16
+; CHECK-NEXT: frame-setup CFI_INSTRUCTION def_cfa_offset 16
+; CHECK-NEXT: SD $x1, $x2, 8 :: (store (s64) into %stack.1)
+; CHECK-NEXT: frame-setup CFI_INSTRUCTION offset $x1, -8
+; CHECK-NEXT: $x2 = frame-setup ADDI $x2, -16
+; CHECK-NEXT: frame-setup CFI_INSTRUCTION def_cfa_offset 16
+; CHECK-NEXT: SD $x1, $x2, 8 :: (store (s64) into %stack.0)
+; CHECK-NEXT: frame-setup CFI_INSTRUCTION offset $x1, -8
+; CHECK-NEXT: BUNDLE implicit-def dead $x6, implicit-def dead $x7, implicit-def dead $x28, implicit-def dead $x29, implicit-def dead $x30, implicit-def dead $x31, implicit-def dead $x1, implicit-def $x2, implicit $x10 {
+; CHECK-NEXT:   KCFI_CHECK $x10, 12345678, implicit-def $x6, implicit-def $x7, implicit-def $x28, implicit-def $x29, implicit-def $x30, implicit-def $x31
+; CHECK-NEXT:   PseudoCALLIndirect $x10, csr_ilp32_lp64, implicit-def dead $x1, implicit-def $x2
+; CHECK-NEXT: }
+; CHECK-NEXT: dead $x1 = LD $x2, 8 :: (load (s64) from %stack.0)
+; CHECK-NEXT: $x2 = frame-destroy ADDI $x2, 16
+; CHECK-NEXT: $x1 = LD $x2, 8 :: (load (s64) from %stack.1)
+; CHECK-NEXT: $x2 = frame-destroy ADDI $x2, 16
+; CHECK-NEXT: PseudoRET
+$x2 = frame-setup ADDI $x2, -16
+frame-setup CFI_INSTRUCTION def_cfa_offset 16
+SD killed $x1, $x2, 8 :: (store (s64) into %stack.0)
+frame-setup CFI_INSTRUCTION offset $x1, -8
+BUNDLE implicit-def $x6, implicit-def $x7, implicit-def $x28, 

[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

> You mean RISC-V specific code or generic code? If generic, I assume we got it 
> from SVE's earlier implementation.

Ah, if SVE has a similar feature then that makes sense.




Comment at: clang/include/clang/Basic/AttrDocs.td:2332
+
+  #if __RISCV_RVV_VLEN_BITS==512
+  typedef vint8m1_t fixed_vint8m1_t 
__attribute__((riscv_rvv_vector_bits(512)));

craig.topper wrote:
> rjmccall wrote:
> > This probably needs a `defined(__RISCV_RVV_VLEN_BITS)` clause, right?  
> > Because the compiler doesn't actually define this macro unless 
> > `-mrvv-vector-bits` is given.
> I guess so. I copied the documentation from the SVE attribute and modified it 
> to RISC-V.
Ah, I see.  Yeah, it's probably wrong there, too.



Comment at: clang/include/clang/Basic/AttrDocs.td:2345
+is enabled under the ``-mrvv-vector-bits`` flag. ``__RISCV_RVV_VLEN_BITS`` can
+only be a power of 2 between 64 and 65536.
+

craig.topper wrote:
> rjmccall wrote:
> > This doesn't describe the actual behavior of the compiler, which is that 
> > it's *ill-formed* to use this attribute except when providing the same 
> > value to `-mrvv-vector-bits`.
> > 
> > Also, this feels like an awkward attempt to also document the 
> > `__RISCV_RVV_VLEN_BITS` macro, which probably ought to be primarily 
> > documented in the command line argument reference for `-mrvv-vector-bits`.
> > This doesn't describe the actual behavior of the compiler, which is that 
> > it's *ill-formed* to use this attribute except when providing the same 
> > value to `-mrvv-vector-bits`.
> 
> I think that means the SVE doc is also incorrect?
> 
> > 
> > Also, this feels like an awkward attempt to also document the 
> > `__RISCV_RVV_VLEN_BITS` macro, which probably ought to be primarily 
> > documented in the command line argument reference for `-mrvv-vector-bits`.
> 
> Ok I'll move it there.
> I think that means the SVE doc is also incorrect?

Yeah.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:207
+Builder.defineMacro("__RISCV_RVV_VLEN_BITS",
+Twine(VScale->first * llvm::RISCV::RVVBitsPerBlock));
 }

craig.topper wrote:
> rjmccall wrote:
> > Is this macro name coming from somewhere specifically?  Because it doesn't 
> > match the normal scheme for RISC-V target macros, which are all lowercase, 
> > and it doesn't match the name of the command line argument it reflects.
> > 
> > Also, why is the computation of this thing so complicated when the 
> > command-line argument is basically a single number?
> > Is this macro name coming from somewhere specifically?  Because it doesn't 
> > match the normal scheme for RISC-V target macros, which are all lowercase, 
> > and it doesn't match the name of the command line argument it reflects.
> 
> I made it up. I'll reconsider it.
> 
> > 
> > Also, why is the computation of this thing so complicated when the 
> > command-line argument is basically a single number?
> 
> The command line is converted to `-mvscale-min=` and `-mvscale-max=` options 
> just like SVE. We divide by `llvm::RISCV::RVVBitsPerBlock` where SVE divides 
> by 128.
> 
> RISC-V does have a concept of minimum vector length through -march already 
> which is checked by getVScaleRange to deal with any disagreement. There's a 
> special value `-mriscv-rvv-vector-bits=zvl` to use the minimum value from 
> -march without needing to repeat the value.
Okay.  So in principle this could be extended to something like a rule where we 
statically check only that the value is within the specified range, and then it 
would be dynamically UB to use a type that's wrong for the actual runtime 
processor?  Maybe that was the idea with SVE but it just never got implemented, 
which is why the documentation looks the way it does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145088

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

[clang] 310ee08 - Reland "[clang-repl] Enable debugging of JIT-ed code."

[Vassil Vassilev via cfe-commits]

Author: Vassil Vassilev
Date: 2023-04-18T18:33:52Z
New Revision: 310ee08d5d45d9388333842a79bf4cef761afddf

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

LOG: Reland "[clang-repl] Enable debugging of JIT-ed code."

Original commit message: "
[clang-repl] Enable debugging of JIT-ed code.

This change follows llvm/llvm-project@21b5ebd and makes use of the jitlink
infrastructure. In order to use this feature inside lldb one needs to run 
the
lldb command: settings set plugin.jit-loader.gdb.enable on

This works currently only on Darwin since jitlink is not a default 
ELF/x86-64
backend yet.

Differential revision: https://reviews.llvm.org/D148481
"

This patch reverts commit e64fbf2cca8c4763a058ba59a48ab8e4b8193028 and adds
the missing library dependencies which caused the initial failure.

Added: 


Modified: 
clang/lib/Interpreter/CMakeLists.txt
clang/lib/Interpreter/IncrementalExecutor.cpp

Removed: 




diff  --git a/clang/lib/Interpreter/CMakeLists.txt 
b/clang/lib/Interpreter/CMakeLists.txt
index c9236248dccb2..721864c0cc1ea 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -3,6 +3,8 @@ set(LLVM_LINK_COMPONENTS
native
Option
OrcJit
+   OrcShared
+   OrcTargetProcess
Support
Target
TargetParser

diff  --git a/clang/lib/Interpreter/IncrementalExecutor.cpp 
b/clang/lib/Interpreter/IncrementalExecutor.cpp
index fdf12dd214148..3f8d60630de41 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.cpp
+++ b/clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -21,11 +21,18 @@
 #include "llvm/ExecutionEngine/Orc/IRCompileLayer.h"
 #include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"
+#include "llvm/ExecutionEngine/Orc/TargetProcess/JITLoaderGDB.h"
 #include "llvm/ExecutionEngine/SectionMemoryManager.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/TargetSelect.h"
 
+// Force linking some of the runtimes that helps attaching to a debugger.
+LLVM_ATTRIBUTE_USED void linkComponents() {
+  llvm::errs() << (void *)_orc_registerJITLoaderGDBWrapper
+   << (void *)_orc_registerJITLoaderGDBAllocAction;
+}
+
 namespace clang {
 
 IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext ,
@@ -37,7 +44,12 @@ 
IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext ,
 
   auto JTMB = JITTargetMachineBuilder(TI.getTriple());
   JTMB.addFeatures(TI.getTargetOpts().Features);
-  if (auto JitOrErr = LLJITBuilder().setJITTargetMachineBuilder(JTMB).create())
+  LLJITBuilder Builder;
+  Builder.setJITTargetMachineBuilder(JTMB);
+  // Enable debugging of JIT'd code (only works on JITLink for ELF and MachO).
+  Builder.setEnableDebuggerSupport(true);
+
+  if (auto JitOrErr = Builder.create())
 Jit = std::move(*JitOrErr);
   else {
 Err = JitOrErr.takeError();



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

[PATCH] D147844: Emit warning when implicit cast to bool happens in an conditional operator expression when used inside an overloaded shift operator expression

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav updated this revision to Diff 514702.
chaitanyav added a comment.

Update release notes about the fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/parentheses.cpp


Index: clang/test/Sema/parentheses.cpp
===
--- clang/test/Sema/parentheses.cpp
+++ clang/test/Sema/parentheses.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -Wparentheses -fsyntax-only -verify %s
-// RUN: %clang_cc1 -Wparentheses -fsyntax-only -fdiagnostics-parseable-fixits 
%s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -Wparentheses -Woverloaded-shift-op-parentheses 
-fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wparentheses -Woverloaded-shift-op-parentheses 
-fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 bool someConditionFunc();
 
@@ -31,14 +31,14 @@
 
 class Stream {
 public:
-  operator int();
+  operator bool();
   Stream <<(int);
   Stream <<(const char*);
   Stream >>(int);
   Stream >>(const char*);
 };
 
-void f(Stream& s, bool b) {
+void f(Stream& s, bool b, int x) {
   (void)(s << b ? "foo" : "bar"); // expected-warning {{operator '?:' has 
lower precedence than '<<'}} \
   // expected-note {{place parentheses around 
the '<<' expression to silence this warning}} \
   // expected-note {{place parentheses around 
the '?:' expression to evaluate it first}}
@@ -62,6 +62,30 @@
   // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:16-[[@LINE-4]]:16}:")"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
   // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:21-[[@LINE-6]]:21}:")"
+
+  void(s << "Test" << x ? "foo" : "bar"); //expected-warning {{operator '?:' 
has lower precedence than '<<'}} \
+ // expected-note {{place parentheses 
around the '<<' expression to silence this warning}} \
+// expected-note {{place parentheses 
around the '?:' expression to evaluate it first}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:8-[[@LINE-3]]:8}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:24}:")"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:23-[[@LINE-5]]:23}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:40-[[@LINE-6]]:40}:")"
+
+  void(s << x == 1 ? "foo": "bar"); //expected-warning {{overloaded operator 
<< has higher precedence than comparison operator}} \
+   //expected-note {{place parentheses around 
the '<<' expression to silence this warning}} \
+  // expected-note {{place parentheses around 
comparison expression to evaluate it first}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:8-[[@LINE-3]]:8}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:14-[[@LINE-4]]:14}:")"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:13-[[@LINE-5]]:13}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:19-[[@LINE-6]]:19}:")"
+
+  void(s << static_cast(x) ? "foo" : "bar"); //expected-warning 
{{operator '?:' has lower precedence than '<<'}} \
+  //expected-note {{place 
parentheses around the '<<' expression to silence this warning}} \
+ //expected-note {{place 
parentheses around the '?:' expression to evaluate it first}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:8-[[@LINE-3]]:8}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:33-[[@LINE-4]]:33}:")"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:13-[[@LINE-5]]:13}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:49-[[@LINE-6]]:49}:")"
 }
 
 struct S {
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9304,7 +9304,7 @@
 
   if (!IsArithmeticBinaryExpr(Condition, , ))
 return;
-  if (!ExprLooksBoolean(CondRHS))
+  if (!ExprLooksBoolean(CondRHS) && !ExprLooksBoolean(Condition))
 return;
 
   // The condition is an arithmetic binary expression, with a right-
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -314,6 +314,9 @@
   (`#61885 `_)
 - Clang constexpr evaluator now treats comparison of [[gnu::weak]]-attributed
   member pointer as an invalid expression.
+- Emit warning when implicit cast to bool happens in an conditional
+  operator expression when used inside an overloaded shift operator expression
+  (`#61943 `_)
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/Sema/parentheses.cpp
===
--- clang/test/Sema/parentheses.cpp

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen marked 9 inline comments as done.
samitolvanen added a comment.

In D148385#4275617 , @jrtc27 wrote:

> I can't help but feel the core of this should be target-independent, with TLI 
> or similar hooks to actually do the target-specific bits?

I agree, that might be a nice improvement. The AArch64 version at least is 
nearly identical to this one, although there's a bit more going on in the X86 
version. I'll take a look.




Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:277
+void RISCVAsmPrinter::LowerKCFI_CHECK(const MachineInstr ) {
+  assert(STI->is64Bit() && "KCFI_CHECK requires RV64");
+

jrtc27 wrote:
> What about this code relies on it being 64-bit?
Good point, this doesn't require much tweaking to also work on RV32. We just 
need to avoid `ADDIW`.



Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:299-300
+  ++NextReg;
+Reg = NextReg++;
+if (Reg > RISCV::X31)
+  report_fatal_error("Unable to find scratch registers for KCFI_CHECK");

nickdesaulniers wrote:
> if (++NextReg > RISCV::X31)
That's not quite the same. We want to check the current register, not the next 
one, which we might not even need. I can certainly not post-increment the 
`NextReg` value if it's confusing.



Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:310
+EmitToStreamer(*OutStreamer, MCInstBuilder(RISCV::ADDI)
+ .addReg(ScratchRegs[0])
+ .addReg(RISCV::X0)

craig.topper wrote:
> craig.topper wrote:
> > Are there two many operands here?
> Err.   "too many"
Yes there are, thanks for noticing.



Comment at: llvm/lib/Target/RISCV/RISCVKCFI.cpp:106-107
+  for (MachineBasicBlock  : MF) {
+for (MachineBasicBlock::instr_iterator MII = MBB.instr_begin(),
+   MIE = MBB.instr_end();
+ MII != MIE; ++MII) {

nickdesaulniers wrote:
> for (MachineInstr  : MBB)
We don't want to skip bundles here, so we have to use `instr_iterator` 
explicitly.



Comment at: llvm/test/CodeGen/RISCV/kcfi.ll:1
+; RUN: llc -mtriple=riscv64 -verify-machineinstrs -riscv-no-aliases < %s | 
FileCheck %s --check-prefix=ASM
+; RUN: llc -mtriple=riscv64 -verify-machineinstrs -stop-after=finalize-isel < 
%s | FileCheck %s --check-prefixes=MIR,ISEL

jrtc27 wrote:
> Please try not to introduce tests with hand-written CHECK lines, but if you 
> have to, try and make them look similar in style to the auto-generated lines.
`update_llc_test_checks.py` doesn't play well with testing function prefix 
data, but I updated the style to match its output, added RV32 (without 
duplicating identical code), and moved the MIR tests to separate files with 
auto-generated checks. Please let me know if there's something specific I'm 
missing here.



Comment at: llvm/utils/gn/secondary/llvm/lib/Target/RISCV/BUILD.gn:81
 "RISCVInstrInfo.cpp",
+"RISCVKCFI.cpp",
 "RISCVMCInstLower.cpp",

craig.topper wrote:
> Isn't this done automatically by the GN Syncbot when CMakeLists.txt is 
> updated?
I remember updating this manually before, but if that's the case, I'm happy to 
drop this line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148385

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

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 514699.
fahadnayyar added a comment.

Fixing some more regression test errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148266

Files:
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/mock-libcxx/lib/libc++.dylib
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
  clang/test/Driver/stdlibxx-isystem.cpp
  clang/test/Tooling/Inputs/mock-libcxx/lib/libc++.dylib

Index: clang/test/Driver/stdlibxx-isystem.cpp
===
--- clang/test/Driver/stdlibxx-isystem.cpp
+++ clang/test/Driver/stdlibxx-isystem.cpp
@@ -5,6 +5,8 @@
 
 // By default, we should search for libc++ next to the driver.
 // RUN: mkdir -p %t/bin
+// RUN: mkdir -p %t/lib
+// RUN: touch %t/lib/libc++.dylib
 // RUN: mkdir -p %t/include/c++/v1
 // RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -1,63 +1,49 @@
 // UNSUPPORTED: system-windows
 
-// General tests that the header search paths for libc++ detected by the driver
-// and passed to CC1 are correct on Darwin platforms.
+// Tests to check that we pass -L /bin/../lib/libc++.dylib to link with the toolchain's libc++.dylib 
+// whenever we are including the toolchain's libc++ headers
 
 // Check without a sysroot and without headers alongside the installation
-// (no include path should be added, and no warning or error).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
-// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s
-// CHECK-LIBCXX-NONE: "-cc1"
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s
 
 // Check with only headers alongside the installation (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: --sysroot="" \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s
-// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
 // RUN:   -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s
-// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 
 // Check with only headers in the sysroot (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s
-// CHECK-LIBCXX-SYSROOT-1: "-cc1"
-// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
-// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s
 
 // Check with both headers in the sysroot and headers alongside the installation
-// (the headers in the toolchain should be preferred over the  headers).
-// Ensure that both 

[PATCH] D148554: [dataflow] Extract arena for Value/StorageLocation out of DataflowAnalysisContext

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added a comment.

Love it! Thanks.




Comment at: clang/include/clang/Analysis/FlowSensitive/Arena.h:129
+} // namespace clang::dataflow
\ No newline at end of file


add newline?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148554

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

[PATCH] D148596: [KMSAN] Enable on SystemZ

[Ilya Leoshkevich via Phabricator via cfe-commits]

iii updated this revision to Diff 514692.
iii added a comment.

- Better explain the ABI situation.
- Extend the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148596

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/test/Instrumentation/MemorySanitizer/SystemZ/basic-kernel.ll

Index: llvm/test/Instrumentation/MemorySanitizer/SystemZ/basic-kernel.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/MemorySanitizer/SystemZ/basic-kernel.ll
@@ -0,0 +1,169 @@
+; RUN: opt < %s -S -mcpu=z13 -msan-kernel=1 -float-abi=soft -passes=msan 2>&1 | FileCheck %s
+
+target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64"
+target triple = "s390x-unknown-linux-gnu"
+
+define void @Store1(ptr %p, i8 %x) sanitize_memory {
+entry:
+  store i8 %x, ptr %p
+  ret void
+}
+
+; CHECK-LABEL: @Store1
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_store_1(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: store i8 {{.+}}, ptr [[SHADOW]], align 1
+; CHECK: ret void
+
+define void @Store2(ptr %p, i16 %x) sanitize_memory {
+entry:
+  store i16 %x, ptr %p
+  ret void
+}
+
+; CHECK-LABEL: @Store2
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_store_2(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: store i16 {{.+}}, ptr [[SHADOW]], align 2
+; CHECK: ret void
+
+define void @Store4(ptr %p, i32 %x) sanitize_memory {
+entry:
+  store i32 %x, ptr %p
+  ret void
+}
+
+; CHECK-LABEL: @Store4
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_store_4(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: store i32 {{.+}}, ptr [[SHADOW]], align 4
+; CHECK: ret void
+
+define void @Store8(ptr %p, i64 %x) sanitize_memory {
+entry:
+  store i64 %x, ptr %p
+  ret void
+}
+
+; CHECK-LABEL: @Store8
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_store_8(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: store i64 {{.+}}, ptr [[SHADOW]], align 8
+; CHECK: ret void
+
+define void @Store16(ptr %p, i128 %x) sanitize_memory {
+entry:
+  store i128 %x, ptr %p
+  ret void
+}
+
+; CHECK-LABEL: @Store16
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_store_n(ptr [[META_PTR]], ptr %p, i64 16)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: store i128 {{.+}}, ptr [[SHADOW]], align 8
+; CHECK: ret void
+
+define i8 @Load1(ptr %p) sanitize_memory {
+entry:
+  %0 = load i8, ptr %p
+  ret i8 %0
+}
+
+; CHECK-LABEL: @Load1
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_load_1(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: [[SHADOW_VAL:%[a-z0-9_]+]] = load i8, ptr [[SHADOW]], align 1
+; CHECK: [[ORIGIN_VAL:%[a-z0-9_]+]] = load i32, ptr [[ORIGIN]], align 4
+; CHECK: store i8 [[SHADOW_VAL]], ptr %retval_shadow, align 8
+; CHECK: store i32 [[ORIGIN_VAL]], ptr %retval_origin, align 4
+; CHECK: ret i8 {{.+}}
+
+define i16 @Load2(ptr %p) sanitize_memory {
+entry:
+  %0 = load i16, ptr %p
+  ret i16 %0
+}
+
+; CHECK-LABEL: @Load2
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_load_2(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue 

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

[Hunter T. via Phabricator via cfe-commits]

StrangeRanger added a comment.

In D148467#4277960 , @MyDeveloperDay 
wrote:

> I'm trying to decide if I put that option in this change or in a separate 
> change (thoughts @HazardyKnusperkeks, @owenpan )

I'm not too privy to your workflow, but if by "in a separate change" you mean 
like "in a different PR", I'd probably say implement it in a different change, 
for the sake of revision history.


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

https://reviews.llvm.org/D148467

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

[PATCH] D148422: [ClangFE] Handle statement expressions properly with CheckAtomicAlignment()

[Jonas Paulsson 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 rG790c9ac529c9: [ClangFE] Handle statement expressions 
properly with CheckAtomicAlignment(). (authored by jonpa).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148422

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/SystemZ/sync-builtins-i128-16Al.c

Index: clang/test/CodeGen/SystemZ/sync-builtins-i128-16Al.c
===
--- clang/test/CodeGen/SystemZ/sync-builtins-i128-16Al.c
+++ clang/test/CodeGen/SystemZ/sync-builtins-i128-16Al.c
@@ -204,3 +204,16 @@
 __int128 f17() {
   return __sync_swap(, Val);
 }
+
+// Test that a statement expression compiles.
+// CHECK-LABEL: @f18(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[T_ADDR:%.*]] = alloca i128, align 8
+// CHECK-NEXT:[[T:%.*]] = load i128, ptr [[TMP0:%.*]], align 8, !tbaa [[TBAA2]]
+// CHECK-NEXT:store i128 [[T]], ptr [[T_ADDR]], align 8, !tbaa [[TBAA2]]
+// CHECK-NEXT:[[TMP1:%.*]] = cmpxchg ptr [[T_ADDR]], i128 [[T]], i128 [[T]] seq_cst seq_cst, align 16
+// CHECK-NEXT:ret void
+//
+void f18(__int128 t) {
+  __sync_bool_compare_and_swap(({int x = 1; }), t, t);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -170,6 +170,21 @@
   return V;
 }
 
+static llvm::Value *CheckAtomicAlignment(CodeGenFunction ,
+ const CallExpr *E) {
+  ASTContext  = CGF.getContext();
+  Address Ptr = CGF.EmitPointerWithAlignment(E->getArg(0));
+  unsigned Bytes = Ptr.getElementType()->isPointerTy()
+   ? Ctx.getTypeSizeInChars(Ctx.VoidPtrTy).getQuantity()
+   : Ptr.getElementType()->getScalarSizeInBits() / 8;
+  unsigned Align = Ptr.getAlignment().getQuantity();
+  if (Align % Bytes != 0) {
+DiagnosticsEngine  = CGF.CGM.getDiags();
+Diags.Report(E->getBeginLoc(), diag::warn_sync_op_misaligned);
+  }
+  return Ptr.getPointer();
+}
+
 /// Utility to insert an atomic instruction based on Intrinsic::ID
 /// and the expression node.
 static Value *MakeBinaryAtomicValue(
@@ -182,7 +197,7 @@
   E->getArg(0)->getType()->getPointeeType()));
   assert(CGF.getContext().hasSameUnqualifiedType(T, E->getArg(1)->getType()));
 
-  llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0));
+  llvm::Value *DestPtr = CheckAtomicAlignment(CGF, E);
   unsigned AddrSpace = DestPtr->getType()->getPointerAddressSpace();
 
   llvm::IntegerType *IntType =
@@ -224,23 +239,9 @@
   return CGF.EmitLoadOfScalar(LV, E->getExprLoc());
 }
 
-static void CheckAtomicAlignment(CodeGenFunction , const CallExpr *E) {
-  ASTContext  = CGF.getContext();
-  Address Ptr = CGF.EmitPointerWithAlignment(E->getArg(0));
-  unsigned Bytes = Ptr.getElementType()->isPointerTy()
-   ? Ctx.getTypeSizeInChars(Ctx.VoidPtrTy).getQuantity()
-   : Ptr.getElementType()->getScalarSizeInBits() / 8;
-  unsigned Align = Ptr.getAlignment().getQuantity();
-  if (Align % Bytes != 0) {
-DiagnosticsEngine  = CGF.CGM.getDiags();
-Diags.Report(E->getBeginLoc(), diag::warn_sync_op_misaligned);
-  }
-}
-
 static RValue EmitBinaryAtomic(CodeGenFunction ,
llvm::AtomicRMWInst::BinOp Kind,
const CallExpr *E) {
-  CheckAtomicAlignment(CGF, E);
   return RValue::get(MakeBinaryAtomicValue(CGF, Kind, E));
 }
 
@@ -252,14 +253,13 @@
const CallExpr *E,
Instruction::BinaryOps Op,
bool Invert = false) {
-  CheckAtomicAlignment(CGF, E);
   QualType T = E->getType();
   assert(E->getArg(0)->getType()->isPointerType());
   assert(CGF.getContext().hasSameUnqualifiedType(T,
   E->getArg(0)->getType()->getPointeeType()));
   assert(CGF.getContext().hasSameUnqualifiedType(T, E->getArg(1)->getType()));
 
-  llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0));
+  llvm::Value *DestPtr = CheckAtomicAlignment(CGF, E);
   unsigned AddrSpace = DestPtr->getType()->getPointerAddressSpace();
 
   llvm::IntegerType *IntType =
@@ -300,9 +300,8 @@
 /// invoke the function EmitAtomicCmpXchgForMSIntrin.
 static Value *MakeAtomicCmpXchgValue(CodeGenFunction , const CallExpr *E,
  bool ReturnBool) {
-  CheckAtomicAlignment(CGF, E);
   QualType T = ReturnBool ? E->getArg(1)->getType() : E->getType();
-  llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0));
+  llvm::Value *DestPtr = CheckAtomicAlignment(CGF, E);
   unsigned AddrSpace = 

[clang] 790c9ac - [ClangFE] Handle statement expressions properly with CheckAtomicAlignment().

[Jonas Paulsson via cfe-commits]

Author: Jonas Paulsson
Date: 2023-04-18T19:33:32+02:00
New Revision: 790c9ac529c9ad0d7e89e3de2041d85cfc411b40

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

LOG: [ClangFE] Handle statement expressions properly with 
CheckAtomicAlignment().

Make CheckAtomicAlignment() return the computed pointer for reuse to avoid
emitting it twice.

Reviewed By: efriedma

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/SystemZ/sync-builtins-i128-16Al.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 1d79d9a146353..ec1698c4117b8 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -170,6 +170,21 @@ static Value *EmitFromInt(CodeGenFunction , 
llvm::Value *V,
   return V;
 }
 
+static llvm::Value *CheckAtomicAlignment(CodeGenFunction ,
+ const CallExpr *E) {
+  ASTContext  = CGF.getContext();
+  Address Ptr = CGF.EmitPointerWithAlignment(E->getArg(0));
+  unsigned Bytes = Ptr.getElementType()->isPointerTy()
+   ? Ctx.getTypeSizeInChars(Ctx.VoidPtrTy).getQuantity()
+   : Ptr.getElementType()->getScalarSizeInBits() / 8;
+  unsigned Align = Ptr.getAlignment().getQuantity();
+  if (Align % Bytes != 0) {
+DiagnosticsEngine  = CGF.CGM.getDiags();
+Diags.Report(E->getBeginLoc(), diag::warn_sync_op_misaligned);
+  }
+  return Ptr.getPointer();
+}
+
 /// Utility to insert an atomic instruction based on Intrinsic::ID
 /// and the expression node.
 static Value *MakeBinaryAtomicValue(
@@ -182,7 +197,7 @@ static Value *MakeBinaryAtomicValue(
   E->getArg(0)->getType()->getPointeeType()));
   assert(CGF.getContext().hasSameUnqualifiedType(T, E->getArg(1)->getType()));
 
-  llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0));
+  llvm::Value *DestPtr = CheckAtomicAlignment(CGF, E);
   unsigned AddrSpace = DestPtr->getType()->getPointerAddressSpace();
 
   llvm::IntegerType *IntType =
@@ -224,23 +239,9 @@ static Value *EmitNontemporalLoad(CodeGenFunction , 
const CallExpr *E) {
   return CGF.EmitLoadOfScalar(LV, E->getExprLoc());
 }
 
-static void CheckAtomicAlignment(CodeGenFunction , const CallExpr *E) {
-  ASTContext  = CGF.getContext();
-  Address Ptr = CGF.EmitPointerWithAlignment(E->getArg(0));
-  unsigned Bytes = Ptr.getElementType()->isPointerTy()
-   ? Ctx.getTypeSizeInChars(Ctx.VoidPtrTy).getQuantity()
-   : Ptr.getElementType()->getScalarSizeInBits() / 8;
-  unsigned Align = Ptr.getAlignment().getQuantity();
-  if (Align % Bytes != 0) {
-DiagnosticsEngine  = CGF.CGM.getDiags();
-Diags.Report(E->getBeginLoc(), diag::warn_sync_op_misaligned);
-  }
-}
-
 static RValue EmitBinaryAtomic(CodeGenFunction ,
llvm::AtomicRMWInst::BinOp Kind,
const CallExpr *E) {
-  CheckAtomicAlignment(CGF, E);
   return RValue::get(MakeBinaryAtomicValue(CGF, Kind, E));
 }
 
@@ -252,14 +253,13 @@ static RValue EmitBinaryAtomicPost(CodeGenFunction ,
const CallExpr *E,
Instruction::BinaryOps Op,
bool Invert = false) {
-  CheckAtomicAlignment(CGF, E);
   QualType T = E->getType();
   assert(E->getArg(0)->getType()->isPointerType());
   assert(CGF.getContext().hasSameUnqualifiedType(T,
   E->getArg(0)->getType()->getPointeeType()));
   assert(CGF.getContext().hasSameUnqualifiedType(T, E->getArg(1)->getType()));
 
-  llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0));
+  llvm::Value *DestPtr = CheckAtomicAlignment(CGF, E);
   unsigned AddrSpace = DestPtr->getType()->getPointerAddressSpace();
 
   llvm::IntegerType *IntType =
@@ -300,9 +300,8 @@ static RValue EmitBinaryAtomicPost(CodeGenFunction ,
 /// invoke the function EmitAtomicCmpXchgForMSIntrin.
 static Value *MakeAtomicCmpXchgValue(CodeGenFunction , const CallExpr *E,
  bool ReturnBool) {
-  CheckAtomicAlignment(CGF, E);
   QualType T = ReturnBool ? E->getArg(1)->getType() : E->getType();
-  llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0));
+  llvm::Value *DestPtr = CheckAtomicAlignment(CGF, E);
   unsigned AddrSpace = DestPtr->getType()->getPointerAddressSpace();
 
   llvm::IntegerType *IntType = llvm::IntegerType::get(
@@ -4045,8 +4044,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
   case Builtin::BI__sync_lock_release_4:
   case Builtin::BI__sync_lock_release_8:
   case Builtin::BI__sync_lock_release_16: {
-CheckAtomicAlignment(*this, E);
-  

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

[MyDeveloperDay via Phabricator via cfe-commits]

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

upload the correct patch file


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

https://reviews.llvm.org/D148467

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -1612,5 +1612,155 @@
   EXPECT_NE("", format("int where b <")); // reduced from crasher
 }
 
+TEST_F(FormatTestCSharp, PropertyWrapping) {
+  FormatStyle Style = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+  Style.BraceWrapping.AfterCSharpProperty = false;
+  verifyFormat("class A\n"
+   "{\n"
+   "string Foo { set; get; }\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Foo { get; set; }\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Foo { get; init; }\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Foo\n"
+   "{\n"
+   "set {\n"
+   "val = value;\n"
+   "}\n"
+   "get {\n"
+   "return value;\n"
+   "}\n"
+   "}\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Foo\n"
+   "{\n"
+   "init {\n"
+   "val = value;\n"
+   "}\n"
+   "get {\n"
+   "return value;\n"
+   "}\n"
+   "}\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar\n"
+   "{\n"
+   "get;\n"
+   "set {\n"
+   "val = value;\n"
+   "}\n"
+   "}\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar\n"
+   "{\n"
+   "get;\n"
+   "init {\n"
+   "val = value;\n"
+   "}\n"
+   "}\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterCSharpProperty = true;
+
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar {\n"
+   "set;\n"
+   "get;\n"
+   "}\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar {\n"
+   "get;\n"
+   "set;\n"
+   "}\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar {\n"
+   "get;\n"
+   "init;\n"
+   "}\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar\n"
+   "{\n"
+   "set\n"
+   "{\n"
+   "val = value;\n"
+   "}\n"
+   "get\n"
+   "{\n"
+   "return value;\n"
+   "}\n"
+   "}\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar\n"
+   "{\n"
+   "init\n"
+   "{\n"
+   "val = value;\n"
+   "}\n"
+   "get\n"
+   "{\n"
+   "return value;\n"
+   "}\n"
+   "}\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar\n"
+   "{\n"
+   "get;\n"
+   "set\n"
+   "{\n"
+   "val = value;\n"
+   "}\n"
+   "}\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar\n"
+   "{\n"
+   "get;\n"
+

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

[MyDeveloperDay via Phabricator via cfe-commits]

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

add init support and fix indentation issue when only one property is defined 
but the is an auto-property


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

https://reviews.llvm.org/D148467

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -1612,5 +1612,93 @@
   EXPECT_NE("", format("int where b <")); // reduced from crasher
 }
 
+TEST_F(FormatTestCSharp, PropertyWrapping) {
+  FormatStyle Style = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+  Style.BraceWrapping.AfterCSharpProperty = false;
+  verifyFormat("class A\n"
+   "{\n"
+   "string Foo { set; get; }\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Foo { get; set; }\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Foo\n"
+   "{\n"
+   "set {\n"
+   "val = value;\n"
+   "}\n"
+   "get {\n"
+   "return value;\n"
+   "}\n"
+   "}\n"
+   "}",
+   Style);
+  /* TODO fix the indentation of get;
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar {\n"
+   "get;\n"
+   "set {\n"
+   "val = value;\n"
+   "}\n"
+   "}\n"
+   "}",
+   Style);
+  */
+
+  Style.BraceWrapping.AfterCSharpProperty = true;
+
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar {\n"
+   "set;\n"
+   "get;\n"
+   "}\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar {\n"
+   "get;\n"
+   "set;\n"
+   "}\n"
+   "}",
+   Style);
+  verifyFormat("class A\n"
+   "{\n"
+   "string Bar\n"
+   "{\n"
+   "set\n"
+   "{\n"
+   "val = value;\n"
+   "}\n"
+   "get\n"
+   "{\n"
+   "return value;\n"
+   "}\n"
+   "}\n"
+   "}",
+   Style);
+  /* TODO fix the indentation of get;
+  verifyFormat("class A\n"
+ "{\n"
+ "string Bar {\n"
+ "get;\n"
+ "set\n"
+ "{\n"
+ "val = value;\n"
+ "}\n"
+ "}\n"
+ "}",
+ Style);
+  */
+}
+
 } // namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -5011,6 +5011,13 @@
   return true;
 }
 
+// Handle `string Foo {{set {} get {}}`.
+if (Style.isCSharp() && Style.BraceWrapping.AfterCSharpProperty &&
+(Line.startsWith(Keywords.kw_get) ||
+ Line.startsWith(Keywords.kw_set))) {
+  return true;
+}
+
 // Don't attempt to interpret struct return types as structs.
 if (Right.isNot(TT_FunctionLBrace)) {
   return (Line.startsWith(tok::kw_class) &&
@@ -5020,6 +5027,16 @@
 }
   }
 
+  // Handle default `string Foo {set;get }`.
+  if (Style.isCSharp() && Style.BraceWrapping.AfterCSharpProperty &&
+  ((Left.isOneOf(tok::l_brace, tok::semi) &&
+Right.isOneOf(Keywords.kw_set, Keywords.kw_get)) ||
+   (Left.is(tok::semi) && Left.Previous &&
+Left.Previous->isOneOf(Keywords.kw_get, Keywords.kw_set) &&
+Right.is(tok::r_brace {
+return true;
+  }
+
   if (Left.is(TT_ObjCBlockLBrace) &&
   Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never) {
 return true;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -178,6 +178,7 @@
 IO.mapOptional("AfterCaseLabel", Wrapping.AfterCaseLabel);
 IO.mapOptional("AfterClass", Wrapping.AfterClass);
 IO.mapOptional("AfterControlStatement", 

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

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

xazax.hun accepted this revision.
xazax.hun added a comment.

I am a bit overloaded at the moment, feel free to commit. I can still add 
comments later that could be addressed in a follow up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146591

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

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

@xazax.hun do you still want to look at this again?
I'm itching a little to get some use out of it :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146591

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

[PATCH] D147016: [PowerPC] Add function pointer alignment to DataLayout

[David Tenty 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 rG8d2e9fc8553c: [PowerPC] Add function pointer alignment to 
DataLayout (authored by daltenty).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147016

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/PPC.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
  llvm/test/CodeGen/PowerPC/pr45301.ll
  llvm/test/ThinLTO/X86/builtin-nostrip-aix.ll

Index: llvm/test/ThinLTO/X86/builtin-nostrip-aix.ll
===
--- llvm/test/ThinLTO/X86/builtin-nostrip-aix.ll
+++ llvm/test/ThinLTO/X86/builtin-nostrip-aix.ll
@@ -30,7 +30,7 @@
 ; CHECK-NM: D __ssp_canary_word
 ; CHECK-NM: T __stack_chk_fail
 
-target datalayout = "E-m:a-p:32:32-i64:64-n32"
+target datalayout = "E-m:a-p:32:32-Fi32-i64:64-n32"
 target triple = "powerpc-ibm-aix-xcoff"
 
 define void @bar() {
Index: llvm/test/CodeGen/PowerPC/pr45301.ll
===
--- llvm/test/CodeGen/PowerPC/pr45301.ll
+++ llvm/test/CodeGen/PowerPC/pr45301.ll
@@ -12,21 +12,21 @@
 ; CHECK-NEXT:bl i
 ; CHECK-NEXT:nop
 ; CHECK-NEXT:addis r4, r2, g@toc@ha
-; CHECK-NEXT:addi r4, r4, g@toc@l
-; CHECK-NEXT:ld r5, 16(r4)
-; CHECK-NEXT:std r5, 16(r3)
-; CHECK-NEXT:ld r6, 0(r4)
-; CHECK-NEXT:std r6, 0(r3)
-; CHECK-NEXT:rldicl r6, r6, 32, 32
-; CHECK-NEXT:ld r7, 8(r4)
+; CHECK-NEXT:addi r5, r4, g@toc@l
+; CHECK-NEXT:ld r6, 16(r5)
+; CHECK-NEXT:std r6, 16(r3)
+; CHECK-NEXT:ld r4, g@toc@l(r4)
+; CHECK-NEXT:std r4, 0(r3)
+; CHECK-NEXT:rldicl r4, r4, 32, 32
+; CHECK-NEXT:ld r7, 8(r5)
 ; CHECK-NEXT:std r7, 8(r3)
-; CHECK-NEXT:ld r7, 24(r4)
+; CHECK-NEXT:ld r7, 24(r5)
 ; CHECK-NEXT:std r7, 24(r3)
-; CHECK-NEXT:ld r4, 32(r4)
-; CHECK-NEXT:std r4, 32(r3)
+; CHECK-NEXT:ld r5, 32(r5)
+; CHECK-NEXT:std r5, 32(r3)
+; CHECK-NEXT:stwbrx r4, 0, r3
 ; CHECK-NEXT:li r4, 20
-; CHECK-NEXT:stwbrx r6, 0, r3
-; CHECK-NEXT:stwbrx r5, r3, r4
+; CHECK-NEXT:stwbrx r6, r3, r4
 ; CHECK-NEXT:addi r1, r1, 112
 ; CHECK-NEXT:ld r0, 16(r1)
 ; CHECK-NEXT:mtlr r0
Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -161,6 +161,17 @@
   if (!is64Bit || T.getOS() == Triple::Lv2)
 Ret += "-p:32:32";
 
+  // If the target ABI uses function descriptors, then the alignment of function
+  // pointers depends on the alignment used to emit the descriptor. Otherwise,
+  // function pointers are aligned to 32 bits because the instructions must be.
+  if ((T.getArch() == Triple::ppc64 && !T.isPPC64ELFv2ABI())) {
+Ret += "-Fi64";
+  } else if (T.isOSAIX()) {
+Ret += is64Bit ? "-Fi64" : "-Fi32";
+  } else {
+Ret += "-Fn32";
+  }
+
   // Note, the alignment values for f64 and i64 on ppc64 in Darwin
   // documentation are wrong; these are correct (i.e. "what gcc does").
   Ret += "-i64:64";
Index: clang/test/CodeGen/target-data.c
===
--- clang/test/CodeGen/target-data.c
+++ clang/test/CodeGen/target-data.c
@@ -88,7 +88,7 @@
 
 // RUN: %clang_cc1 -triple powerpc64-lv2 -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=PS3
-// PS3: target datalayout = "E-m:e-p:32:32-i64:64-n32:64"
+// PS3: target datalayout = "E-m:e-p:32:32-Fi64-i64:64-n32:64"
 
 // RUN: %clang_cc1 -triple i686-nacl -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=I686-NACL
@@ -120,43 +120,43 @@
 
 // RUN: %clang_cc1 -triple powerpc-unknown -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=PPC
-// PPC: target datalayout = "E-m:e-p:32:32-i64:64-n32"
+// PPC: target datalayout = "E-m:e-p:32:32-Fn32-i64:64-n32"
 
 // RUN: %clang_cc1 -triple powerpcle-unknown -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=PPCLE
-// PPCLE: target datalayout = "e-m:e-p:32:32-i64:64-n32"
+// PPCLE: target datalayout = "e-m:e-p:32:32-Fn32-i64:64-n32"
 
 // RUN: %clang_cc1 -triple powerpc64-freebsd -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=PPC64-FREEBSD
-// PPC64-FREEBSD: target datalayout = "E-m:e-i64:64-n32:64"
+// PPC64-FREEBSD: target datalayout = "E-m:e-Fn32-i64:64-n32:64"
 
 // RUN: %clang_cc1 -triple powerpc64le-freebsd -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=PPC64LE-FREEBSD
-// PPC64LE-FREEBSD: target datalayout = "e-m:e-i64:64-n32:64"
+// PPC64LE-FREEBSD: target datalayout = "e-m:e-Fn32-i64:64-n32:64"
 
 // RUN: %clang_cc1 -triple powerpc64-linux -o - -emit-llvm %s | \
 // RUN: FileCheck 

[clang] 8d2e9fc - [PowerPC] Add function pointer alignment to DataLayout

[David Tenty via cfe-commits]

Author: David Tenty
Date: 2023-04-18T13:00:27-04:00
New Revision: 8d2e9fc8553c91b1f22e82ffc2c00020f40bb0a2

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

LOG: [PowerPC] Add function pointer alignment to DataLayout

The alignment of function pointers was added to the Datalayout by
D57335 but currently is unset for the Power target. This will cause us
to compute a conservative minimum alignment of one if places like
Value::getPointerAlignment.

This patch implements the function pointer alignment in the Datalayout
for the Power backend and Power targets in clang, so we can query the
value for a particular Power target.

We come up with the correct value one of two ways:

- If the target uses function descriptor objects (i.e. ELFv1 & AIX ABIs),
  then a function pointer points to the descriptor, so use the alignment
  we would emit the descriptor with.
- If the target doesn't use function descriptor objects (i.e. ELFv2), a
  function pointer points to the global entry point, so use the minimum
  alignment for code on Power (i.e. 4-bytes).

Reviewed By: nemanjai

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

Added: 


Modified: 
clang/lib/Basic/Targets/OSTargets.h
clang/lib/Basic/Targets/PPC.h
clang/test/CodeGen/target-data.c
llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
llvm/test/CodeGen/PowerPC/pr45301.ll
llvm/test/ThinLTO/X86/builtin-nostrip-aix.ll

Removed: 




diff  --git a/clang/lib/Basic/Targets/OSTargets.h 
b/clang/lib/Basic/Targets/OSTargets.h
index ea21f7eafaba1..cb2a68f27edaa 100644
--- a/clang/lib/Basic/Targets/OSTargets.h
+++ b/clang/lib/Basic/Targets/OSTargets.h
@@ -519,7 +519,7 @@ class LLVM_LIBRARY_VISIBILITY PS3PPUTargetInfo : public 
OSTargetInfo {
 this->IntMaxType = TargetInfo::SignedLongLong;
 this->Int64Type = TargetInfo::SignedLongLong;
 this->SizeType = TargetInfo::UnsignedInt;
-this->resetDataLayout("E-m:e-p:32:32-i64:64-n32:64");
+this->resetDataLayout("E-m:e-p:32:32-Fi64-i64:64-n32:64");
   }
 };
 

diff  --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index 07a8a3aefca7f..8564fc0346406 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -363,11 +363,11 @@ class LLVM_LIBRARY_VISIBILITY PPC32TargetInfo : public 
PPCTargetInfo {
   PPC32TargetInfo(const llvm::Triple , const TargetOptions )
   : PPCTargetInfo(Triple, Opts) {
 if (Triple.isOSAIX())
-  resetDataLayout("E-m:a-p:32:32-i64:64-n32");
+  resetDataLayout("E-m:a-p:32:32-Fi32-i64:64-n32");
 else if (Triple.getArch() == llvm::Triple::ppcle)
-  resetDataLayout("e-m:e-p:32:32-i64:64-n32");
+  resetDataLayout("e-m:e-p:32:32-Fn32-i64:64-n32");
 else
-  resetDataLayout("E-m:e-p:32:32-i64:64-n32");
+  resetDataLayout("E-m:e-p:32:32-Fn32-i64:64-n32");
 
 switch (getTriple().getOS()) {
 case llvm::Triple::Linux:
@@ -418,19 +418,23 @@ class LLVM_LIBRARY_VISIBILITY PPC64TargetInfo : public 
PPCTargetInfo {
 
 if (Triple.isOSAIX()) {
   // TODO: Set appropriate ABI for AIX platform.
-  DataLayout = "E-m:a-i64:64-n32:64";
+  DataLayout = "E-m:a-Fi64-i64:64-n32:64";
   LongDoubleWidth = 64;
   LongDoubleAlign = DoubleAlign = 32;
   LongDoubleFormat = ::APFloat::IEEEdouble();
 } else if ((Triple.getArch() == llvm::Triple::ppc64le)) {
-  DataLayout = "e-m:e-i64:64-n32:64";
+  DataLayout = "e-m:e-Fn32-i64:64-n32:64";
   ABI = "elfv2";
 } else {
-  DataLayout = "E-m:e-i64:64-n32:64";
-  if (Triple.isPPC64ELFv2ABI())
+  DataLayout = "E-m:e";
+  if (Triple.isPPC64ELFv2ABI()) {
 ABI = "elfv2";
-  else
+DataLayout += "-Fn32";
+  } else {
 ABI = "elfv1";
+DataLayout += "-Fi64";
+  }
+  DataLayout += "-i64:64-n32:64";
 }
 
 if (Triple.isOSFreeBSD() || Triple.isOSOpenBSD() || Triple.isMusl()) {

diff  --git a/clang/test/CodeGen/target-data.c 
b/clang/test/CodeGen/target-data.c
index e5ee17bc38a7f..a7650c88df201 100644
--- a/clang/test/CodeGen/target-data.c
+++ b/clang/test/CodeGen/target-data.c
@@ -88,7 +88,7 @@
 
 // RUN: %clang_cc1 -triple powerpc64-lv2 -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=PS3
-// PS3: target datalayout = "E-m:e-p:32:32-i64:64-n32:64"
+// PS3: target datalayout = "E-m:e-p:32:32-Fi64-i64:64-n32:64"
 
 // RUN: %clang_cc1 -triple i686-nacl -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=I686-NACL
@@ -120,43 +120,43 @@
 
 // RUN: %clang_cc1 -triple powerpc-unknown -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=PPC
-// PPC: target datalayout = "E-m:e-p:32:32-i64:64-n32"
+// PPC: target datalayout = "E-m:e-p:32:32-Fn32-i64:64-n32"
 
 // RUN: %clang_cc1 -triple powerpcle-unknown -o - -emit-llvm %s | 

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.



>>   public Foo {
>>   set;
>>   get;
>>   }
>
> At least from my experience, the getter is specified before the setter, 
> though I'm unsure how important this is in your eyes.

Lets hold off the idea of swapping `set;get` around from this patch, but this 
is something that I think we could do..  (using the FormatTokenLexer.cpp to 
swap trivial `set;get` into `get;set` but lets leave that for another feature, 
as it would be code altering)

honeslty I think we need another option  maybe

enum AllowShortCSharpProperties {

  Leave,
  Never,
  Empty,
  Always

}

**Leave **= Don't touch them
**Never **=  always break  (braces follow AfterCSharpProperty

  set;
  get;
  
  set { 
val = value; 
  }
  get { 
return = value; 
  }

**Empty**  = only one the same line when empty

  set; get
  
  set { 
 val = value; 
  }
  get { 
return = value;
  }

**Always **=  always short form for trivial

  set; get;
  
  set {  val = value; }
  get {  return = value; }

I'm trying to decide if I put that option in this change or in a seperate 
change (thoughts @HazardyKnusperkeks, @owenpan )

FYI, I've solved the indention issue in my branch which has been broken forever 
from what I can tell.

  verifyFormat("class A\n"
   "{\n"
   "string Bar {\n"
   "get;\n"
   "set\n"
   "{\n"
   "val = value;\n"
   "}\n"
   "}\n"
   "}",
   Style);




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

https://reviews.llvm.org/D148467

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

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

[Hunter T. via Phabricator via cfe-commits]

StrangeRanger added a comment.

In D148467#4272231 , @MyDeveloperDay 
wrote:

> for default `set;get` or `get;set` for when `AfterCSharpProperty` is true,
>
>   public Foo {
>   set;
>   get;
>   }

At least from my experience, the getter is specified before the setter, though 
I'm unsure how important this is in your eyes.
If we assume the above sentence is always true, and the brace doesn't break 
after the property name, the following are the options in formatting that I've 
seen:

When autoproperty is used:

  public Foo {
  get;
  set;
  }
  
  // OR
  
  public FooTwo { set; get; }  // What I prefer.

When a non-autoproperty is used:

  /* When only one thing is provided in the getter and setter. */
  
  public Foo {
  get { return _next; }  
  set { _next = value; }
  }  // What I prefer.
  
  // OR
  
  public Foo {
  get => return _next; 
  set => _next = value;
  }  // Another option that I like (which really doesn't apply to this code 
change).
  
  // OR
  
  public Foo {
  get 
  { 
  return _next; 
  }  
  
  set 
  { 
  _next = value; 
  }
  }
  
  // --
  
  /* When more than one thing is provided in the getter and setter. */
  
  public Foo {
  get 
  { 
  // ...
  return _next; 
  }  
  
  set 
  { 
  _next = value;
  // ...
  }
  }

Please note that the above examples *slightly* ignore the different variations 
in how the braces could be set (new line or not).

I hope this input is enough/helps. Please let me know if you'd like any 
clarification.


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

https://reviews.llvm.org/D148467

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

[PATCH] D148021: [Headers][doc] Add FMA intrinsic descriptions

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I chose to leave the "for each element" cases as-is, but I will keep your 
comments in mind as I go through other intrinsics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148021

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

[PATCH] D148021: [Headers][doc] Add FMA intrinsic descriptions

[Paul Robinson 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 rG0905c567f0c7: [Headers][doc] Add FMA intrinsic descriptions 
(authored by probinson).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D148021?vs=512461=514663#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148021

Files:
  clang/lib/Headers/fmaintrin.h

Index: clang/lib/Headers/fmaintrin.h
===
--- clang/lib/Headers/fmaintrin.h
+++ clang/lib/Headers/fmaintrin.h
@@ -18,192 +18,756 @@
 #define __DEFAULT_FN_ATTRS128 __attribute__((__always_inline__, __nodebug__, __target__("fma"), __min_vector_width__(128)))
 #define __DEFAULT_FN_ATTRS256 __attribute__((__always_inline__, __nodebug__, __target__("fma"), __min_vector_width__(256)))
 
+/// Computes a multiply-add of 128-bit vectors of [4 x float].
+///For each element, computes  (__A * __B) + __C .
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VFMADD213PS instruction.
+///
+/// \param __A
+///A 128-bit vector of [4 x float] containing the multiplicand.
+/// \param __B
+///A 128-bit vector of [4 x float] containing the multiplier.
+/// \param __C
+///A 128-bit vector of [4 x float] containing the addend.
+/// \returns A 128-bit vector of [4 x float] containing the result.
 static __inline__ __m128 __DEFAULT_FN_ATTRS128
 _mm_fmadd_ps(__m128 __A, __m128 __B, __m128 __C)
 {
   return (__m128)__builtin_ia32_vfmaddps((__v4sf)__A, (__v4sf)__B, (__v4sf)__C);
 }
 
+/// Computes a multiply-add of 128-bit vectors of [2 x double].
+///For each element, computes  (__A * __B) + __C .
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VFMADD213PD instruction.
+///
+/// \param __A
+///A 128-bit vector of [2 x double] containing the multiplicand.
+/// \param __B
+///A 128-bit vector of [2 x double] containing the multiplier.
+/// \param __C
+///A 128-bit vector of [2 x double] containing the addend.
+/// \returns A 128-bit [2 x double] vector containing the result.
 static __inline__ __m128d __DEFAULT_FN_ATTRS128
 _mm_fmadd_pd(__m128d __A, __m128d __B, __m128d __C)
 {
   return (__m128d)__builtin_ia32_vfmaddpd((__v2df)__A, (__v2df)__B, (__v2df)__C);
 }
 
+/// Computes a scalar multiply-add of the single-precision values in the
+///low 32 bits of 128-bit vectors of [4 x float].
+/// \code
+/// result[31:0] = (__A[31:0] * __B[31:0]) + __C[31:0]
+/// result[127:32] = __A[127:32]
+/// \endcode
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VFMADD213SS instruction.
+///
+/// \param __A
+///A 128-bit vector of [4 x float] containing the multiplicand in the low
+///32 bits.
+/// \param __B
+///A 128-bit vector of [4 x float] containing the multiplier in the low
+///32 bits.
+/// \param __C
+///A 128-bit vector of [4 x float] containing the addend in the low
+///32 bits.
+/// \returns A 128-bit vector of [4 x float] containing the result in the low
+///32 bits and a copy of \a __A[127:32] in the upper 96 bits.
 static __inline__ __m128 __DEFAULT_FN_ATTRS128
 _mm_fmadd_ss(__m128 __A, __m128 __B, __m128 __C)
 {
   return (__m128)__builtin_ia32_vfmaddss3((__v4sf)__A, (__v4sf)__B, (__v4sf)__C);
 }
 
+/// Computes a scalar multiply-add of the double-precision values in the
+///low 64 bits of 128-bit vectors of [2 x double].
+/// \code
+/// result[63:0] = (__A[63:0] * __B[63:0]) + __C[63:0]
+/// result[127:64] = __A[127:64]
+/// \endcode
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VFMADD213SD instruction.
+///
+/// \param __A
+///A 128-bit vector of [2 x double] containing the multiplicand in the low
+///64 bits.
+/// \param __B
+///A 128-bit vector of [2 x double] containing the multiplier in the low
+///64 bits.
+/// \param __C
+///A 128-bit vector of [2 x double] containing the addend in the low
+///64 bits.
+/// \returns A 128-bit vector of [2 x double] containing the result in the low
+///64 bits and a copy of \a __A[127:64] in the upper 64 bits.
 static __inline__ __m128d __DEFAULT_FN_ATTRS128
 _mm_fmadd_sd(__m128d __A, __m128d __B, __m128d __C)
 {
   return (__m128d)__builtin_ia32_vfmaddsd3((__v2df)__A, (__v2df)__B, (__v2df)__C);
 }
 
+/// Computes a multiply-subtract of 128-bit vectors of [4 x float].
+///For each element, computes  (__A * __B) - __C .
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VFMSUB213PS instruction.
+///
+/// \param __A
+///A 128-bit vector of [4 x float] containing the multiplicand.
+/// \param __B
+///A 128-bit vector of [4 x float] containing the multiplier.
+/// \param __C
+///A 128-bit vector of [4 x float] containing the subtrahend.
+/// \returns A 128-bit vector of [4 x float] containing the 

[clang] 0905c56 - [Headers][doc] Add FMA intrinsic descriptions

[Paul Robinson via cfe-commits]

Author: Paul Robinson
Date: 2023-04-18T09:31:09-07:00
New Revision: 0905c567f0c7328780c727b470a92cda1ee54697

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

LOG: [Headers][doc] Add FMA intrinsic descriptions

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

Added: 


Modified: 
clang/lib/Headers/fmaintrin.h

Removed: 




diff  --git a/clang/lib/Headers/fmaintrin.h b/clang/lib/Headers/fmaintrin.h
index d889b7c5e2705..ea832fac4f992 100644
--- a/clang/lib/Headers/fmaintrin.h
+++ b/clang/lib/Headers/fmaintrin.h
@@ -18,192 +18,756 @@
 #define __DEFAULT_FN_ATTRS128 __attribute__((__always_inline__, __nodebug__, 
__target__("fma"), __min_vector_width__(128)))
 #define __DEFAULT_FN_ATTRS256 __attribute__((__always_inline__, __nodebug__, 
__target__("fma"), __min_vector_width__(256)))
 
+/// Computes a multiply-add of 128-bit vectors of [4 x float].
+///For each element, computes  (__A * __B) + __C .
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VFMADD213PS instruction.
+///
+/// \param __A
+///A 128-bit vector of [4 x float] containing the multiplicand.
+/// \param __B
+///A 128-bit vector of [4 x float] containing the multiplier.
+/// \param __C
+///A 128-bit vector of [4 x float] containing the addend.
+/// \returns A 128-bit vector of [4 x float] containing the result.
 static __inline__ __m128 __DEFAULT_FN_ATTRS128
 _mm_fmadd_ps(__m128 __A, __m128 __B, __m128 __C)
 {
   return (__m128)__builtin_ia32_vfmaddps((__v4sf)__A, (__v4sf)__B, 
(__v4sf)__C);
 }
 
+/// Computes a multiply-add of 128-bit vectors of [2 x double].
+///For each element, computes  (__A * __B) + __C .
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VFMADD213PD instruction.
+///
+/// \param __A
+///A 128-bit vector of [2 x double] containing the multiplicand.
+/// \param __B
+///A 128-bit vector of [2 x double] containing the multiplier.
+/// \param __C
+///A 128-bit vector of [2 x double] containing the addend.
+/// \returns A 128-bit [2 x double] vector containing the result.
 static __inline__ __m128d __DEFAULT_FN_ATTRS128
 _mm_fmadd_pd(__m128d __A, __m128d __B, __m128d __C)
 {
   return (__m128d)__builtin_ia32_vfmaddpd((__v2df)__A, (__v2df)__B, 
(__v2df)__C);
 }
 
+/// Computes a scalar multiply-add of the single-precision values in the
+///low 32 bits of 128-bit vectors of [4 x float].
+/// \code
+/// result[31:0] = (__A[31:0] * __B[31:0]) + __C[31:0]
+/// result[127:32] = __A[127:32]
+/// \endcode
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VFMADD213SS instruction.
+///
+/// \param __A
+///A 128-bit vector of [4 x float] containing the multiplicand in the low
+///32 bits.
+/// \param __B
+///A 128-bit vector of [4 x float] containing the multiplier in the low
+///32 bits.
+/// \param __C
+///A 128-bit vector of [4 x float] containing the addend in the low
+///32 bits.
+/// \returns A 128-bit vector of [4 x float] containing the result in the low
+///32 bits and a copy of \a __A[127:32] in the upper 96 bits.
 static __inline__ __m128 __DEFAULT_FN_ATTRS128
 _mm_fmadd_ss(__m128 __A, __m128 __B, __m128 __C)
 {
   return (__m128)__builtin_ia32_vfmaddss3((__v4sf)__A, (__v4sf)__B, 
(__v4sf)__C);
 }
 
+/// Computes a scalar multiply-add of the double-precision values in the
+///low 64 bits of 128-bit vectors of [2 x double].
+/// \code
+/// result[63:0] = (__A[63:0] * __B[63:0]) + __C[63:0]
+/// result[127:64] = __A[127:64]
+/// \endcode
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VFMADD213SD instruction.
+///
+/// \param __A
+///A 128-bit vector of [2 x double] containing the multiplicand in the low
+///64 bits.
+/// \param __B
+///A 128-bit vector of [2 x double] containing the multiplier in the low
+///64 bits.
+/// \param __C
+///A 128-bit vector of [2 x double] containing the addend in the low
+///64 bits.
+/// \returns A 128-bit vector of [2 x double] containing the result in the low
+///64 bits and a copy of \a __A[127:64] in the upper 64 bits.
 static __inline__ __m128d __DEFAULT_FN_ATTRS128
 _mm_fmadd_sd(__m128d __A, __m128d __B, __m128d __C)
 {
   return (__m128d)__builtin_ia32_vfmaddsd3((__v2df)__A, (__v2df)__B, 
(__v2df)__C);
 }
 
+/// Computes a multiply-subtract of 128-bit vectors of [4 x float].
+///For each element, computes  (__A * __B) - __C .
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VFMSUB213PS instruction.
+///
+/// \param __A
+///A 128-bit vector of [4 x float] containing the multiplicand.
+/// \param __B
+///A 128-bit vector of [4 x float] containing the multiplier.
+/// \param __C
+///A 128-bit vector of [4 x float] containing the subtrahend.
+/// 

[PATCH] D147626: [clang] Do not crash when initializing union with flexible array member

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:808
   unsigned NumElems = numStructUnionElements(ILE->getType());
-  if (RDecl->hasFlexibleArrayMember())
+  if (!RDecl->isUnion() && RDecl->hasFlexibleArrayMember())
 ++NumElems;

Fznamznon wrote:
> aaron.ballman wrote:
> > Fznamznon wrote:
> > > Fznamznon wrote:
> > > > aaron.ballman wrote:
> > > > > Fznamznon wrote:
> > > > > > shafik wrote:
> > > > > > > Fznamznon wrote:
> > > > > > > > Just for some context, numStructUnionElements checks that there 
> > > > > > > > is a flexible array member and returns 
> > > > > > > > number_of_initializable_fields-1 for structs. For unions it 
> > > > > > > > just returns 1 or 0, so flexible array member caused adding one 
> > > > > > > > more element to initlistexpr that was never properly handled.
> > > > > > > > 
> > > > > > > > Instead of doing this change, we could probably never enter 
> > > > > > > > initialization since the record (union) declaration is not 
> > > > > > > > valid, but that is not the case even for other types of errors 
> > > > > > > > in code, for example, I've tried declaring field of struct with 
> > > > > > > > a typo:
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > struct { cha x[]; } r = {1}; 
> > > > > > > > ```
> > > > > > > > Initialization is still performed by clang.
> > > > > > > > Also, it seems MSVC considers flexible array member inside 
> > > > > > > > union as valid, so the test code is probably not always invalid.
> > > > > > > I am not sure what to think here, looking at gcc documentation 
> > > > > > > for this extension: 
> > > > > > > https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html 
> > > > > > > 
> > > > > > > and using the following code:
> > > > > > > 
> > > > > > > ```
> > > > > > > struct f1 {
> > > > > > >   int x; int y[];
> > > > > > > } f1 = { 1, { 2, 3, 4 } }; // #1
> > > > > > > 
> > > > > > > struct f2 {
> > > > > > >   struct f1 f1; int data[3];
> > > > > > > } f2 = { { 1 }, { 2, 3, 4 } }; // #2
> > > > > > > 
> > > > > > > struct { char x[]; } r = {1};  // #3
> > > > > > > ```
> > > > > > > 
> > > > > > > gcc rejects 2 and 3 even though 2 comes from their documentation. 
> > > > > > > Clang warns on 2 and MSVC rejects 2
> > > > > > > 
> > > > > > > CC @aaron.ballman @rsmith 
> > > > > > Yes, I also had a feeling that we probably need to refine how these 
> > > > > > extensions are supported by clang, that is probably a bit out of 
> > > > > > scope of the fix though.
> > > > > The GCC extension differs based on C vs C++: 
> > > > > https://godbolt.org/z/E14Yz37To
> > > > > As does the extension in Clang, but differently than GCC: 
> > > > > https://godbolt.org/z/zYznaYPf5
> > > > > 
> > > > > So I agree that there's work to be done on this extension, but it's 
> > > > > outside of the scope of the crash fix for this patch. For this patch, 
> > > > > GCC rejects allowing a flexible array member in a union in both C and 
> > > > > C++, but it looks like Clang rejects in C and perhaps accepts in C++: 
> > > > > https://godbolt.org/z/bTsPn3G7b So how about we add a C++ test case 
> > > > > for this as well -- if that still crashes, that should be fixed, but 
> > > > > if the code is accepted, we should either decide we want to start 
> > > > > rejecting it or we should ensure the codegen for it is correct.
> > > > While experimenting with C++ test, I've noticed the following thing:
> > > > ```
> > > > union { char x[]; } r = {0}; // when in global scope generates no IR 
> > > > (without my patch crashes)
> > > > 
> > > > void foo() {
> > > >   union { char x[]; } r = {0}; // fails with error "initialization of 
> > > > flexible array member is not allowed" in both C/C++, no crash even 
> > > > without my patch
> > > > }
> > > > 
> > > > union A {char x[]; };
> > > > A a = {0}; // crashes even with my patch but with different assertion 
> > > > when trying -emit-llvm
> > > > void foo() {
> > > >   A a = {0}; // fails with error "initialization of flexible array 
> > > > member is not allowed" in both C and C++, no crash even without my patch
> > > > }
> > > > ```
> > > > 
> > > > It is not entirely clear to me why the behavior different for function 
> > > > and TU scope. gcc always gives an error about flexible array in union, 
> > > > no matter how it is used. Also, it is strange that I'm not seeing the 
> > > > same "initialization of flexible array member is not allowed" error 
> > > > message for structs, just for unions. I'm thinking that we don't really 
> > > > have proper codegen for the code that I'm trying to fix and we should 
> > > > reject the code like gcc does. MSVC is fine with all though - 
> > > > https://godbolt.org/z/aoarEzd56 .
> > > > 
> > > > WDYT?
> > > Ping?
> > Ouch, that's a rather confused situation, isn't it? I don't have the 
> > impression we actually support this extension and we do not document 
> > support for it in the language extensions 

[PATCH] D147743: [Clang][NFC] Rename methods/vars to reflect their real usage

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/Index/IndexBody.cpp:426
+return IndexCtx.handleReference(FD, D.getFieldLoc(), Parent,
+ParentDC, SymbolRoleSet(), {}, E);
+  }

nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147743

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

[PATCH] D148639: [NFC][clang] Fix static analyzer concerns about AUTO_CAUSES_COPY

[Soumi Manna via Phabricator via cfe-commits]

Manna created this revision.
Manna added reviewers: tahonermann, aaron.ballman.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp, 
donat.nagy, Szelethus, a.sidorin, baloghadamsoftware, kristof.beyls.
Herald added a reviewer: NoQ.
Herald added a project: All.
Manna requested review of this revision.
Herald added a project: clang.

Reported by Coverity:

AUTO_CAUSES_COPY
Unnecessary object copies can affect performance

1. Inside "SemaInit.cpp" file, in 
isVarOnPath(llvm::​SmallVectorImpl<::​IndirectLocalPathEntry> &, 
clang::​VarDecl *): Using the auto keyword without an & causes the copy of an 
object of type IndirectLocalPathEntry

2. Inside "ODRHash.cpp" file, in 
clang::​ODRHash::​AddCXXRecordDecl(clang::​CXXRecordDecl const *): Using the 
auto keyword without an & causes the copy of an object of type CXXBaseSpecifier.

3. Inside "CGExpr.cpp" file, in 
clang::​CodeGen::​CodeGenFunction::​EmitCfiCheckFail(): Using the auto keyword 
without an & causes the copy of an object of type pair.

4. Inside "ASTReader.cpp" file, in clang::​ASTReader::​finishPendingActions(): 
Using the auto keyword without an & causes the copy of an object of type pair.

5. Inside "ExprEngine.cpp" file, in 
clang::​ento::​ExprEngine::​removeDead(clang::​ento::​ExplodedNode *, 
clang::​ento::​ExplodedNodeSet &, clang::​Stmt const *, clang::​LocationContext 
const *, clang::​Stmt const *, clang::​ProgramPoint::​Kind): Using the auto 
keyword without an & causes the copy of an object of type pair.

6. Inside "CGVTables.cpp" file, *in 
clang::​CodeGen::​CodeGenModule::​EmitVTableTypeMetadata(clang::​CXXRecordDecl 
const *, llvm::​GlobalVariable *, clang::​VTableLayout const &): Using the auto 
keyword without an & causes the copy of an object of type pair.

7. Inside "ExprConstant.cpp" file, in EvaluateArgs(llvm::​ArrayRef, ::​CallRef, ::​EvalInfo &, clang::​FunctionDecl 
const *, bool): Using the auto keyword without an & causes the copy of an 
object of type ParamIdx.

8. Inside "ARM.cpp" file, in 
clang::​targets::​ARMTargetInfo::​initFeatureMap(llvm::​StringMap &, clang::​DiagnosticsEngine &, llvm::​StringRef, 
std::​vector, 
std::​allocator>, std::​allocator, std::​allocator>>> const &): Using the auto 
keyword without an & causes the copy of an object of type StringRef.

9. Inside "Tokens.cpp" file, in 
clang::​syntax::​TokenBuffer::​dumpForTests[abi:cxx11](): Using the auto 
keyword without an & causes the copy of an object of type DenseMapPair.

10. Inside "TargetID.cpp" file, in 
clang::​getCanonicalTargetID[abi:cxx11](llvm::​StringRef, 
llvm::​StringMap const &): Using the auto keyword 
without an & causes the copy of an object of type pair.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148639

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ODRHash.cpp
  clang/lib/Basic/TargetID.cpp
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/Tooling/Syntax/Tokens.cpp

Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -986,7 +986,7 @@
   OS << "\n";
 
   std::vector Keys;
-  for (auto F : Files)
+  for (const auto  : Files)
 Keys.push_back(F.first);
   llvm::sort(Keys);
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1050,7 +1050,7 @@
   const StackFrameContext *SFC = LC ? LC->getStackFrame() : nullptr;
   SymbolReaper SymReaper(SFC, ReferenceStmt, SymMgr, getStoreManager());
 
-  for (auto I : CleanedState->get()) {
+  for (const auto  : CleanedState->get()) {
 if (SymbolRef Sym = I.second.getAsSymbol())
   SymReaper.markLive(Sym);
 if (const MemRegion *MR = I.second.getAsRegion())
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -9409,7 +9409,7 @@
   /*ErrorOnTagTypeMismatch =*/true);
   if (Ctx.IsEquivalent(ExtensionsPair.first, ExtensionsPair.second)) {
 // Merge redeclared ivars with their predecessors.
-for (auto IvarPair : DuplicateIvars) {
+for (const auto  : DuplicateIvars) {
   ObjCIvarDecl *Ivar = IvarPair.first, *PrevIvar = IvarPair.second;
   // Change semantic DeclContext but keep the lexical one.
   Ivar->setDeclContextsImpl(PrevIvar->getDeclContext(),
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -7168,7 +7168,7 @@
 }
 
 static bool 

[PATCH] D148583: [clang-scan-deps] Add clang-scan-deps to llvm-driver build

[Alex Brachet 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 rGc2423a334585: [clang-scan-deps] Add clang-scan-deps to 
llvm-driver build (authored by abrachet).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148583

Files:
  clang/tools/clang-scan-deps/CMakeLists.txt
  clang/tools/clang-scan-deps/ClangScanDeps.cpp
  llvm/utils/gn/secondary/clang/tools/clang-scan-deps/BUILD.gn
  utils/bazel/llvm-project-overlay/clang/BUILD.bazel


Index: utils/bazel/llvm-project-overlay/clang/BUILD.bazel
===
--- utils/bazel/llvm-project-overlay/clang/BUILD.bazel
+++ utils/bazel/llvm-project-overlay/clang/BUILD.bazel
@@ -2251,9 +2251,18 @@
 td_srcs = [ "//llvm:include/llvm/Option/OptParser.td" ],
 )
 
+expand_template(
+name = "clang-scan-deps-main",
+out = "clang-scan-deps-driver.cpp",
+substitutions = {
+"@TOOL_NAME@": "clang_scan_deps",
+},
+template = "//llvm:cmake/modules/llvm-driver-template.cpp.in",
+)
+
 cc_binary(
 name = "clang-scan-deps",
-srcs = glob(["tools/clang-scan-deps/*.cpp"]),
+srcs = glob(["tools/clang-scan-deps/*.cpp"]) + 
["clang-scan-deps-driver.cpp"],
 stamp = 0,
 deps = [
 ":driver",
Index: llvm/utils/gn/secondary/clang/tools/clang-scan-deps/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/tools/clang-scan-deps/BUILD.gn
+++ llvm/utils/gn/secondary/clang/tools/clang-scan-deps/BUILD.gn
@@ -1,3 +1,4 @@
+import("//llvm/utils/gn/build/driver_executable.gni")
 import("//llvm/utils/TableGen/tablegen.gni")
 
 tablegen("Opts") {
@@ -5,7 +6,7 @@
   args = [ "-gen-opt-parser-defs" ]
 }
 
-executable("clang-scan-deps") {
+driver_executable("clang-scan-deps") {
   configs += [ "//llvm/utils/gn/build:clang_code" ]
   deps = [
 ":Opts",
Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -21,6 +21,7 @@
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Support/LLVMDriver.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ThreadPool.h"
@@ -657,9 +658,9 @@
 // generating P1689 format, trying to generate the compilation database
 // form specified command line after the positional parameter "--".
 static std::unique_ptr
-getCompilationDataBase(int argc, const char **argv, std::string ) 
{
+getCompilationDataBase(int argc, char **argv, std::string ) {
   llvm::InitLLVM X(argc, argv);
-  ParseArgs(argc, const_cast(argv));
+  ParseArgs(argc, argv);
 
   if (!CompilationDB.empty())
 return tooling::JSONCompilationDatabase::loadFromFile(
@@ -674,7 +675,7 @@
 
   // Trying to get the input file, the output file and the command line options
   // from the positional parameter "--".
-  const char **DoubleDash = std::find(argv, argv + argc, StringRef("--"));
+  char **DoubleDash = std::find(argv, argv + argc, StringRef("--"));
   if (DoubleDash == argv + argc) {
 llvm::errs() << "The command line arguments is required after '--' in "
 "P1689 per file mode.";
@@ -746,7 +747,7 @@
   FEOpts.Inputs[0].getFile(), OutputFile, CommandLine);
 }
 
-int main(int argc, const char **argv) {
+int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
   std::string ErrorMessage;
   std::unique_ptr Compilations =
   getCompilationDataBase(argc, argv, ErrorMessage);
Index: clang/tools/clang-scan-deps/CMakeLists.txt
===
--- clang/tools/clang-scan-deps/CMakeLists.txt
+++ clang/tools/clang-scan-deps/CMakeLists.txt
@@ -14,6 +14,7 @@
 
   DEPENDS
   ScanDepsOptsTableGen
+  GENERATE_DRIVER
   )
 
 set(CLANG_SCAN_DEPS_LIB_DEPS


Index: utils/bazel/llvm-project-overlay/clang/BUILD.bazel
===
--- utils/bazel/llvm-project-overlay/clang/BUILD.bazel
+++ utils/bazel/llvm-project-overlay/clang/BUILD.bazel
@@ -2251,9 +2251,18 @@
 td_srcs = [ "//llvm:include/llvm/Option/OptParser.td" ],
 )
 
+expand_template(
+name = "clang-scan-deps-main",
+out = "clang-scan-deps-driver.cpp",
+substitutions = {
+"@TOOL_NAME@": "clang_scan_deps",
+},
+template = "//llvm:cmake/modules/llvm-driver-template.cpp.in",
+)
+
 cc_binary(
 name = "clang-scan-deps",
-srcs = glob(["tools/clang-scan-deps/*.cpp"]),
+srcs = glob(["tools/clang-scan-deps/*.cpp"]) + ["clang-scan-deps-driver.cpp"],
 stamp = 0,
 deps = [
 ":driver",
Index: 

[PATCH] D148355: [analyzer] Fix comparison logic in ArrayBoundCheckerV2

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

steakhal added a comment.

This subject haunted us for quite some time now, and there is more behind what 
it seems at first.

If I'm not mistaken, this patch is pretty much what is in D86874 
 except that it doesn't give up and still 
checks the upper-bound in this edge case - which is good.

I had other attempts at fixing this, from a different angle though, such as in 
D88359  - where the idea was to do the 
//simplification// in the math domain (so not in the c++ language model). 
During this process, we would have the opportunity of explaining what 
assumptions were taken during the process where we finally constrain the (root, 
aka. `x`) symbol. Actually, these missing notes (explanations) are the reason 
why this checker is `alpha`. But yes, the other reason is this FP that you 
fixed.
Do you plan to explore ways to improve the readability of the reports 
explaining the constraints in the future?

At SonarSource, we had a fix for this issue for ~1.5 years now with great 
success.F27176964: sonarsource-CPP-2376.patch 

We decided to keep it downstream, due to the failure of similar attempts :D
Our implementation and your have a lot in common.

The only difference (I think) is that we detect this case:

  void alwaysOverflow(unsigned long long s) {
int x[20];
x[s + 21] = 0; // Ours warn!
   // Hmm, it should work for `x + 20` as well.
  }

I haven't done an extensive differential analysis between this and our patches.
Do you think it would make sense to consider both implementations and forge 
them? Would you be up for that?
We could also say of course that we fix the FP first as a baby step.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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

[clang] e64fbf2 - Revert "[clang-repl] Enable debugging of JIT-ed code."

[Vassil Vassilev via cfe-commits]

Author: Vassil Vassilev
Date: 2023-04-18T13:41:38Z
New Revision: e64fbf2cca8c4763a058ba59a48ab8e4b8193028

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

LOG: Revert "[clang-repl] Enable debugging of JIT-ed code."

This reverts commit 1e4891e681c51f3e2a724b4ac4e42380ec4a9be1.

Some bots complain about undefined llvm_orc_registerJITLoaderGDBAllocAction.

Added: 


Modified: 
clang/lib/Interpreter/IncrementalExecutor.cpp

Removed: 




diff  --git a/clang/lib/Interpreter/IncrementalExecutor.cpp 
b/clang/lib/Interpreter/IncrementalExecutor.cpp
index 3f8d60630de41..fdf12dd214148 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.cpp
+++ b/clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -21,18 +21,11 @@
 #include "llvm/ExecutionEngine/Orc/IRCompileLayer.h"
 #include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"
-#include "llvm/ExecutionEngine/Orc/TargetProcess/JITLoaderGDB.h"
 #include "llvm/ExecutionEngine/SectionMemoryManager.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/TargetSelect.h"
 
-// Force linking some of the runtimes that helps attaching to a debugger.
-LLVM_ATTRIBUTE_USED void linkComponents() {
-  llvm::errs() << (void *)_orc_registerJITLoaderGDBWrapper
-   << (void *)_orc_registerJITLoaderGDBAllocAction;
-}
-
 namespace clang {
 
 IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext ,
@@ -44,12 +37,7 @@ 
IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext ,
 
   auto JTMB = JITTargetMachineBuilder(TI.getTriple());
   JTMB.addFeatures(TI.getTargetOpts().Features);
-  LLJITBuilder Builder;
-  Builder.setJITTargetMachineBuilder(JTMB);
-  // Enable debugging of JIT'd code (only works on JITLink for ELF and MachO).
-  Builder.setEnableDebuggerSupport(true);
-
-  if (auto JitOrErr = Builder.create())
+  if (auto JitOrErr = LLJITBuilder().setJITTargetMachineBuilder(JTMB).create())
 Jit = std::move(*JitOrErr);
   else {
 Err = JitOrErr.takeError();



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

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

[Stefan Gränitz via Phabricator via cfe-commits]

sgraenitz added a comment.

In D141824#4274461 , @argentite wrote:

> We should probably also address the lack of linker issue as well. Should we 
> go for a precompiled dynamic library file? There seems to be some "precedent" 
> of this in other tests.

Yes, since yaml2obj can't do it and the test only runs on Linux anyway, it 
seems reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141824

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

[PATCH] D147743: [Clang][NFC] Rename methods/vars to reflect their real usage

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147743

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

[PATCH] D148573: Port -fsanitize=function to AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

One other small thing. The docs page 
https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html would need the 
supported architectures updating:

  -fsanitize=function: Indirect call of a function through a function pointer 
of the wrong type (Darwin/Linux, C++ and x86/x86_64 only).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148573

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

[PATCH] D148094: [DRAFT][clang][CodeGen] Break up TargetInfo.cpp [6/6]

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

Kindly ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148094

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

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Hi folks!

We have a setup in which clang (more specifically clang-tools) is always built 
from a version close to HEAD and libcxx is fetched from user's checkout of the 
codebase (which can lag behind HEAD for ~a month).
So the 1 week gap between 
https://github.com/llvm/llvm-project/commit/380b6a13dad61e1d90e68fdd128af5dc177db3e1
 and this patch is not enough for us to prevent most of the breakages 
(clang-tools we have cannot work on checkouts that are not synced past this CL 
as they hit the name conflict).

Hence is it possible to revert this change and land it couple weeks later or 
add the token behind a PP-directive (looking at the way token is introduced 
having this optionally available seems to be hard though)?

also it would be nice to have general policies about changes that might break 
backward compat and how long to wait (i guess we would be more careful if the 
libcxx symbol was actually used in a released version). do we already have any 
guidance around that so that we can be prepared for those in the future?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

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

[PATCH] D148573: Port -fsanitize=function to AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

As it stands I think this may have problems with -mbranch-protection. In that 
case we'll need a `BTI c` to be the target of the indirect branch. I'm guessing 
something like:

  _Z3funv
  BTI C ; In hint space
  B . + 8 
  .word .L__llvm_rtti_proxy-_Z3funv

Otherwise when the indirect call is made then it will fail on a system with BTI 
enabled.

Not too sure how much of a problem this is for the implementation. The `BTI c` 
can't be used as a signature, I guess the code in the caller could check the 
value at `_z3funv + 4` . The feature could be marked as incompatible with 
`-mbranch-protection`. I guess it may not work well with patchable functions 
either.

I would expect the emitGlobalConstant to emit data. This would be visible in 
the object file as we'd have:

  $d
  
  
  $x
  instructions.

At the moment I don't think that this would affect anything except 
disassemblers, and the LLD cortex-a53 eratta work around which excludes $d from 
the disassembly. It is something that it could be worth fixing, expecially if 
there is a `BTI C` involved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148573

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