[PATCH] D144638: [lit] Detect Inconsistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

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

To be clear: I've not yet addressed @jhenderson's comments above, yet, which 
may require changes to the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

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

[PATCH] D142410: [AArch64] ARMv8.5-A implies both FEAT_SB and FEAT_SSBS

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

@philipp.tomsich Reverse Ping. What are your plans for this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142410

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

[PATCH] D146141: [ARM] Use FPUKind enum instead of unsigned

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary 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/D146141/new/

https://reviews.llvm.org/D146141

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

[PATCH] D145563: [AArch64] Assembly Support for FEAT_GCS/FEAT_CHK

[Sam Elliott via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
lenary marked an inline comment as done.
Closed by commit rGcb7fb737858c: [AArch64] Assembly Support for 
FEAT_GCS/FEAT_CHK (authored by lenary).

Changed prior to commit:
  https://reviews.llvm.org/D145563?vs=503280=505433#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145563

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  llvm/include/llvm/TargetParser/AArch64TargetParser.h
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64SystemOperands.td
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/test/MC/AArch64/armv9.4a-chk.s
  llvm/test/MC/AArch64/armv9.4a-gcs.s
  llvm/test/MC/AArch64/directive-arch_extension-negative.s
  llvm/test/MC/AArch64/directive-arch_extension.s
  llvm/test/MC/Disassembler/AArch64/armv9.4a-chk.txt
  llvm/test/MC/Disassembler/AArch64/armv9.4a-gcs.txt
  llvm/unittests/TargetParser/TargetParserTest.cpp

Index: llvm/unittests/TargetParser/TargetParserTest.cpp
===
--- llvm/unittests/TargetParser/TargetParserTest.cpp
+++ llvm/unittests/TargetParser/TargetParserTest.cpp
@@ -1600,7 +1600,7 @@
   AArch64::AEK_B16B16,  AArch64::AEK_SMEF16F16, AArch64::AEK_CSSC,
   AArch64::AEK_RCPC3,   AArch64::AEK_THE,   AArch64::AEK_D128,
   AArch64::AEK_LSE128,  AArch64::AEK_SPECRES2,  AArch64::AEK_RASv2,
-  AArch64::AEK_ITE,
+  AArch64::AEK_ITE, AArch64::AEK_GCS,
   };
 
   std::vector Features;
@@ -1671,6 +1671,7 @@
   EXPECT_TRUE(llvm::is_contained(Features, "+lse128"));
   EXPECT_TRUE(llvm::is_contained(Features, "+specres2"));
   EXPECT_TRUE(llvm::is_contained(Features, "+ite"));
+  EXPECT_TRUE(llvm::is_contained(Features, "+gcs"));
 
   // Assuming we listed every extension above, this should produce the same
   // result. (note that AEK_NONE doesn't have a name so it won't be in the
@@ -1793,6 +1794,7 @@
   {"pmuv3", "nopmuv3", "+perfmon", "-perfmon"},
   {"predres2", "nopredres2", "+specres2", "-specres2"},
   {"rasv2", "norasv2", "+rasv2", "-rasv2"},
+  {"gcs", "nogcs", "+gcs", "-gcs"},
   };
 
   for (unsigned i = 0; i < std::size(ArchExt); i++) {
Index: llvm/test/MC/Disassembler/AArch64/armv9.4a-gcs.txt
===
--- /dev/null
+++ llvm/test/MC/Disassembler/AArch64/armv9.4a-gcs.txt
@@ -0,0 +1,90 @@
+# RUN: llvm-mc -triple=aarch64 -mattr +gcs -disassemble %s 2> %t | FileCheck %s
+
+[0x00,0x25,0x18,0xd5]
+[0x01,0x25,0x38,0xd5]
+// CHECK: msr GCSCR_EL1, x0
+// CHECK: mrs x1, GCSCR_EL1
+
+[0x22,0x25,0x18,0xd5]
+[0x23,0x25,0x38,0xd5]
+// CHECK: msr GCSPR_EL1, x2
+// CHECK: mrs x3, GCSPR_EL1
+
+[0x44,0x25,0x18,0xd5]
+[0x45,0x25,0x38,0xd5]
+// CHECK: msr GCSCRE0_EL1, x4
+// CHECK: mrs x5, GCSCRE0_EL1
+
+[0x26,0x25,0x1b,0xd5]
+[0x27,0x25,0x3b,0xd5]
+// CHECK: msr GCSPR_EL0, x6
+// CHECK: mrs x7, GCSPR_EL0
+
+[0x0a,0x25,0x1c,0xd5]
+[0x0b,0x25,0x3c,0xd5]
+// CHECK: msr GCSCR_EL2, x10
+// CHECK: mrs x11, GCSCR_EL2
+
+[0x2c,0x25,0x1c,0xd5]
+[0x2d,0x25,0x3c,0xd5]
+// CHECK: msr GCSPR_EL2, x12
+// CHECK: mrs x13, GCSPR_EL2
+
+[0x0e,0x25,0x1d,0xd5]
+[0x0f,0x25,0x3d,0xd5]
+// CHECK: msr GCSCR_EL12, x14
+// CHECK: mrs x15, GCSCR_EL12
+
+[0x30,0x25,0x1d,0xd5]
+[0x31,0x25,0x3d,0xd5]
+// CHECK: msr GCSPR_EL12, x16
+// CHECK: mrs x17, GCSPR_EL12
+
+[0x12,0x25,0x1e,0xd5]
+[0x13,0x25,0x3e,0xd5]
+// CHECK: msr GCSCR_EL3, x18
+// CHECK: mrs x19, GCSCR_EL3
+
+[0x34,0x25,0x1e,0xd5]
+[0x35,0x25,0x3e,0xd5]
+// CHECK: msr GCSPR_EL3, x20
+// CHECK: mrs x21, GCSPR_EL3
+
+[0x55,0x77,0x0b,0xd5]
+// CHECK: gcsss1 x21
+
+[0x76,0x77,0x2b,0xd5]
+// CHECK: gcsss2x22
+
+[0x19,0x77,0x0b,0xd5]
+// CHECK: gcspushm x25
+
+[0x3f,0x77,0x2b,0xd5]
+// CHECK: gcspopm
+
+[0x39,0x77,0x2b,0xd5]
+// CHECK: gcspopmx25
+
+[0x7f,0x22,0x03,0xd5]
+// CHECK: gcsbdsync
+
+[0x7a,0x0f,0x1f,0xd9]
+// CHECK: gcsstr   x26, x27
+
+[0xfa,0x0f,0x1f,0xd9]
+// CHECK: gcsstr   x26, sp
+
+[0x7a,0x1f,0x1f,0xd9]
+// CHECK: gcssttr  x26, x27
+
+[0xfa,0x1f,0x1f,0xd9]
+// CHECK: gcssttr  x26, sp
+
+[0x9f,0x77,0x08,0xd5]
+// CHECK: gcspushx
+
+[0xbf,0x77,0x08,0xd5]
+// CHECK: gcspopcx
+
+[0xdf,0x77,0x08,0xd5]
+// CHECK: gcspopx
Index: llvm/test/MC/Disassembler/AArch64/armv9.4a-chk.txt
===
--- /dev/null
+++ llvm/test/MC/Disassembler/AArch64/armv9.4a-chk.txt
@@ -0,0 +1,8 @@
+# RUN: llvm-mc -triple=aarch64 -mattr=+v8.9a -disassemble %s 2> %t | FileCheck %s
+# RUN: llvm-mc -triple=aarch64 -mattr=+v9.4a -disassemble %s 2> %t | FileCheck %s
+# RUN: llvm-mc -triple=aarch64 -mattr=+chk   -disassemble %s 2> %t | FileCheck %s
+# RUN: 

[PATCH] D145563: [AArch64] Assembly Support for FEAT_GCS/FEAT_CHK

[Sam Elliott via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:1106
+
+def : TokenAlias<"DSYNC", "dsync">;
+

john.brawn wrote:
> It would make more sense to put this immediately after GCSB_DSYNC.
This will be moved in the version that I land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145563

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

[PATCH] D144638: [lit] Detect Inconsistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

lenary marked an inline comment as done.
lenary added a comment.

@jhenderson @int3 I think I have addressed your feedback - are you happy for me 
to land this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

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

[PATCH] D145563: [AArch64] Assembly Support for FEAT_GCS/FEAT_CHK

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

The CHKFEAT instruction here is not exactly as described in 
https://developer.arm.com/downloads/-/exploration-tools today - but the next 
release of that description will be updated with the new syntax.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145563

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

[PATCH] D145563: [AArch64] Assembly Support for FEAT_GCS/FEAT_CHK

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
lenary requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This implements support for two new 2022 A-profile extensions:

- FEAT_CHK - Check Feature Status Extension
- FEAT_GCS - Guarded Control Stacks

FEAT_CHK is mandatory from armv8.0-a, but is in the hint space so
there's no clang command-line flag for it, and we only print the hint as
`chkfeat x16` at v8.9a and above, to be compatible when using a
non-integrated assembler that might not yet know about the extension.

FEAT_GCS is optional from armv9.4-a onwards. It is enabled using `+gcs`
in a clang `-march=` or `-mcpu=` option string, or using a
`.arch_extension gcs` assembly directive.

This patch includes changes by Ties Stuij, Tomas Matheson, and Keith
Walker.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145563

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  llvm/include/llvm/TargetParser/AArch64TargetParser.h
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64SystemOperands.td
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/test/MC/AArch64/armv9.4a-chk.s
  llvm/test/MC/AArch64/armv9.4a-gcs.s
  llvm/test/MC/AArch64/directive-arch_extension-negative.s
  llvm/test/MC/AArch64/directive-arch_extension.s
  llvm/test/MC/Disassembler/AArch64/armv9.4a-chk.txt
  llvm/test/MC/Disassembler/AArch64/armv9.4a-gcs.txt
  llvm/unittests/TargetParser/TargetParserTest.cpp

Index: llvm/unittests/TargetParser/TargetParserTest.cpp
===
--- llvm/unittests/TargetParser/TargetParserTest.cpp
+++ llvm/unittests/TargetParser/TargetParserTest.cpp
@@ -1600,7 +1600,7 @@
   AArch64::AEK_B16B16,  AArch64::AEK_SMEF16F16, AArch64::AEK_CSSC,
   AArch64::AEK_RCPC3,   AArch64::AEK_THE,   AArch64::AEK_D128,
   AArch64::AEK_LSE128,  AArch64::AEK_SPECRES2,  AArch64::AEK_RASv2,
-  AArch64::AEK_ITE,
+  AArch64::AEK_ITE, AArch64::AEK_GCS,
   };
 
   std::vector Features;
@@ -1671,6 +1671,7 @@
   EXPECT_TRUE(llvm::is_contained(Features, "+lse128"));
   EXPECT_TRUE(llvm::is_contained(Features, "+specres2"));
   EXPECT_TRUE(llvm::is_contained(Features, "+ite"));
+  EXPECT_TRUE(llvm::is_contained(Features, "+gcs"));
 
   // Assuming we listed every extension above, this should produce the same
   // result. (note that AEK_NONE doesn't have a name so it won't be in the
@@ -1793,6 +1794,7 @@
   {"pmuv3", "nopmuv3", "+perfmon", "-perfmon"},
   {"predres2", "nopredres2", "+specres2", "-specres2"},
   {"rasv2", "norasv2", "+rasv2", "-rasv2"},
+  {"gcs", "nogcs", "+gcs", "-gcs"},
   };
 
   for (unsigned i = 0; i < std::size(ArchExt); i++) {
Index: llvm/test/MC/Disassembler/AArch64/armv9.4a-gcs.txt
===
--- /dev/null
+++ llvm/test/MC/Disassembler/AArch64/armv9.4a-gcs.txt
@@ -0,0 +1,90 @@
+# RUN: llvm-mc -triple=aarch64 -mattr +gcs -disassemble %s 2> %t | FileCheck %s
+
+[0x00,0x25,0x18,0xd5]
+[0x01,0x25,0x38,0xd5]
+// CHECK: msr GCSCR_EL1, x0
+// CHECK: mrs x1, GCSCR_EL1
+
+[0x22,0x25,0x18,0xd5]
+[0x23,0x25,0x38,0xd5]
+// CHECK: msr GCSPR_EL1, x2
+// CHECK: mrs x3, GCSPR_EL1
+
+[0x44,0x25,0x18,0xd5]
+[0x45,0x25,0x38,0xd5]
+// CHECK: msr GCSCRE0_EL1, x4
+// CHECK: mrs x5, GCSCRE0_EL1
+
+[0x26,0x25,0x1b,0xd5]
+[0x27,0x25,0x3b,0xd5]
+// CHECK: msr GCSPR_EL0, x6
+// CHECK: mrs x7, GCSPR_EL0
+
+[0x0a,0x25,0x1c,0xd5]
+[0x0b,0x25,0x3c,0xd5]
+// CHECK: msr GCSCR_EL2, x10
+// CHECK: mrs x11, GCSCR_EL2
+
+[0x2c,0x25,0x1c,0xd5]
+[0x2d,0x25,0x3c,0xd5]
+// CHECK: msr GCSPR_EL2, x12
+// CHECK: mrs x13, GCSPR_EL2
+
+[0x0e,0x25,0x1d,0xd5]
+[0x0f,0x25,0x3d,0xd5]
+// CHECK: msr GCSCR_EL12, x14
+// CHECK: mrs x15, GCSCR_EL12
+
+[0x30,0x25,0x1d,0xd5]
+[0x31,0x25,0x3d,0xd5]
+// CHECK: msr GCSPR_EL12, x16
+// CHECK: mrs x17, GCSPR_EL12
+
+[0x12,0x25,0x1e,0xd5]
+[0x13,0x25,0x3e,0xd5]
+// CHECK: msr GCSCR_EL3, x18
+// CHECK: mrs x19, GCSCR_EL3
+
+[0x34,0x25,0x1e,0xd5]
+[0x35,0x25,0x3e,0xd5]
+// CHECK: msr GCSPR_EL3, x20
+// CHECK: mrs x21, GCSPR_EL3
+
+[0x55,0x77,0x0b,0xd5]
+// CHECK: gcsss1 x21
+
+[0x76,0x77,0x2b,0xd5]
+// CHECK: gcsss2x22
+
+[0x19,0x77,0x0b,0xd5]
+// CHECK: gcspushm x25
+
+[0x3f,0x77,0x2b,0xd5]
+// CHECK: gcspopm
+
+[0x39,0x77,0x2b,0xd5]
+// CHECK: gcspopmx25
+
+[0x7f,0x22,0x03,0xd5]
+// CHECK: gcsbdsync
+
+[0x7a,0x0f,0x1f,0xd9]
+// CHECK: gcsstr   x26, x27
+
+[0xfa,0x0f,0x1f,0xd9]
+// CHECK: gcsstr   x26, sp
+
+[0x7a,0x1f,0x1f,0xd9]
+// CHECK: gcssttr  x26, x27
+
+[0xfa,0x1f,0x1f,0xd9]
+// CHECK: gcssttr  x26, sp
+
+[0x9f,0x77,0x08,0xd5]
+// CHECK: gcspushx
+
+[0xbf,0x77,0x08,0xd5]
+// CHECK: gcspopcx
+
+[0xdf,0x77,0x08,0xd5]
+// 

[PATCH] D144638: [lit] Detect Inconsistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/utils/lit/lit/llvm/config.py:190
+return False
+if "1995" not in touch_res_out:
+return False

michaelplatings wrote:
> This could end up matching the wrong part of the string, for example if the 
> temporary file happened to have 1995 in its name. A regex match for 
> '\b1995\b' would be more reliable.
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

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

[PATCH] D144638: [lit] Detect Inconsistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 501838.
lenary marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

Files:
  clang/test/Modules/prune.m
  lld/test/COFF/lto-cache.ll
  lld/test/ELF/lto/cache.ll
  lld/test/MachO/lto-cache.ll
  lld/test/wasm/lto/cache.ll
  llvm/test/ThinLTO/X86/cache.ll
  llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -5,6 +5,7 @@
 import subprocess
 import sys
 import errno
+import tempfile
 
 import lit.util
 from lit.llvm.subst import FindTool
@@ -155,6 +156,40 @@
 self.with_environment(
 'DYLD_INSERT_LIBRARIES', gmalloc_path_str)
 
+if self._has_consistent_atime():
+features.add('consistent-atime')
+
+# Some tests use `touch` to set the access time for a file, and expect no
+# other processes to change this during the test run. This is not always the
+# case, so we have a way to disable these tests on systems where the access
+# might not be preserved.
+def _has_consistent_atime(self):
+# NetBSD: noatime mounts currently inhibit 'touch -a' updates.
+if platform.system() == 'NetBSD':
+return False
+
+# Windows: the last access time is disabled by default in the OS, and
+# the check below is written in terms of unix utilities (touch, ls),
+# which will not work on this platform.
+if platform.system() == 'Windows':
+return False
+
+# Other Platforms: Try to use `touch -a` to set the atime, and then to
+# read it with `ls`. If they don't match, or either process errors, then
+# don't claim that atime is consistent.
+with tempfile.NamedTemporaryFile() as f:
+# Specific date in the past on purpose, based on what is attempted
+# in the tests that do the same thing.
+(_, try_touch_err) = self.get_process_output(["touch", "-a", "-t", "199505050555.55", f.name])
+if try_touch_err != "":
+return False
+
+(touch_res_out, touch_res_err) = self.get_process_output(["ls", "-lu", f.name])
+if touch_res_err != "":
+return False
+
+return re.search(r'\b1995\b', touch_res_out) is not None
+
 def _find_git_windows_unix_tools(self, tools_needed):
 assert(sys.platform == 'win32')
 if sys.version_info.major >= 3:
Index: llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
===
--- llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
+++ llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
@@ -1,7 +1,5 @@
 # Note: ls -lu prints the accessed timestamp
-# NetBSD: noatime mounts currently inhibit 'touch -a' updates
-# Windows: the last access time is disabled by default in the OS
-# UNSUPPORTED: system-netbsd, system-windows
+# REQUIRES: consistent-atime
 
 # Preserve dates when stripping to an output file.
 # RUN: yaml2obj %s -o %t.1.o
Index: llvm/test/ThinLTO/X86/cache.ll
===
--- llvm/test/ThinLTO/X86/cache.ll
+++ llvm/test/ThinLTO/X86/cache.ll
@@ -1,5 +1,4 @@
-; NetBSD: noatime mounts currently inhibit 'touch -a' updates
-; UNSUPPORTED: system-netbsd
+; REQUIRES: consistent-atime
 
 ; The .noindex suffix for output dir is to prevent Spotlight on macOS from
 ; indexing it.
Index: lld/test/wasm/lto/cache.ll
===
--- lld/test/wasm/lto/cache.ll
+++ lld/test/wasm/lto/cache.ll
@@ -1,8 +1,6 @@
 ; RUN: opt -module-hash -module-summary %s -o %t.o
 ; RUN: opt -module-hash -module-summary %p/Inputs/cache.ll -o %t2.o
-; NetBSD: noatime mounts currently inhibit 'touch' from updating atime
-; Windows: no 'touch' command.
-; UNSUPPORTED: system-netbsd, system-windows
+; REQUIRES: consistent-atime
 
 ; RUN: rm -Rf %t.cache && mkdir %t.cache
 ; Create two files that would be removed by cache pruning due to age.
Index: lld/test/MachO/lto-cache.ll
===
--- lld/test/MachO/lto-cache.ll
+++ lld/test/MachO/lto-cache.ll
@@ -1,6 +1,5 @@
 ; REQUIRES: x86
-; NetBSD: noatime mounts currently inhibit 'touch' from updating atime
-; UNSUPPORTED: system-netbsd
+; REQUIRES: consistent-atime
 
 ; RUN: rm -rf %t; split-file %s %t
 ; RUN: opt -module-hash -module-summary %t/foo.ll -o %t/foo.o
Index: lld/test/ELF/lto/cache.ll
===
--- lld/test/ELF/lto/cache.ll
+++ lld/test/ELF/lto/cache.ll
@@ -1,6 +1,5 @@
 ; REQUIRES: x86
-; NetBSD: noatime mounts 

[PATCH] D144638: [lit] Detect Inconsistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D144638#4155217 , @michaelplatings 
wrote:

> I see some of these tests previously had `UNSUPPORTED: system-netbsd` but not 
> `UNSUPPORTED: system-windows` - do you know why?

I'm not entirely sure why, but I have some ideas:

- rGa680ea2c720751a3d724fd8282515924acb3bd32 
 disabled 
the llvm-objdump test - I don't quite know why it didn't disable the 
ThinLTO/x86/cache.ll test (which did exist at the time).
- rG929ce27cc77dd1fbf909ef4d42614eee9d11880e 
, 
rG29935acdb10679f27f028cf9dff1971c839dc51d 
, and 
rG92dc7dce4a6f117a497ced1650bc48e5b658f0ea 
 are the 
netbsd disables, which were done all approximately at the same time, across 
clang, llvm and lld.

It looks like the pruning logic for both LTO and Clang modules is the same, 
using `llvm::sys::fs::file_status::getLastAccessedTime()` (but, this of course 
has to use the underlying system information), which is:

- On windows, atime will be zero if unsupported: If the underlying file system 
does not support last access time, this member is zero. 
https://learn.microsoft.com/en-us/windows/win32/api/minwinbase/ns-minwinbase-win32_find_dataw
 which should be a long time in the past.
- The mac, netbsd and linux docs for `noatime` just say something to the effect 
of "the access time is never updated", not the value that it reads as if you 
have noatime.

I've actually just done a more thorough search, and there are other tests that 
use `touch -a` (a few in clang/test/PCH, some others in clang/test/Modules). I 
presume this comes down to logic as to whether the code uses 
`getLastAccessedTime` which seems to be used:

- in the modules cache prune logic (pruneModuleCache in 
clang/lib/Frontend/CompilerInstance.cpp)
- in the LTO cache pruning logic (llvm::pruneCache)
- in FileCollector, as used by Modules 
(llvm/include/llvm/Support/FileCollector.h)
- in FilePermissionsApplier, as used by llvm-dwarfutil and llvm-objcopy 
(declared in llvm/include/llvm/Support/FileUtilities.h)

I cannot really see any correlation between the logic under test, and the use 
of `touch -a` or `touch -a -m` vs just `touch`. I'm not sure what the right 
"fix" should be any more. I guess I'm just happy for everyone who is not 
running into this issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

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

[PATCH] D144638: [lit] Detect Inconsistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

lenary marked 2 inline comments as done.
lenary added a comment.

Hopefully this addresses the feedback so far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

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

[PATCH] D144638: [lit] Detect Inconsistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

lenary retitled this revision from "[lit] Detect Consistent File Access Times" 
to "[lit] Detect Inconsistent File Access Times".
lenary edited the summary of this revision.
lenary updated this revision to Diff 500742.
Herald added a subscriber: krytarowski.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

Files:
  clang/test/Modules/prune.m
  lld/test/COFF/lto-cache.ll
  lld/test/ELF/lto/cache.ll
  lld/test/MachO/lto-cache.ll
  lld/test/wasm/lto/cache.ll
  llvm/test/ThinLTO/X86/cache.ll
  llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -5,6 +5,7 @@
 import subprocess
 import sys
 import errno
+import tempfile
 
 import lit.util
 from lit.llvm.subst import FindTool
@@ -155,6 +156,42 @@
 self.with_environment(
 'DYLD_INSERT_LIBRARIES', gmalloc_path_str)
 
+if self._has_consistent_atime():
+features.add('consistent-atime')
+
+# Some tests use `touch` to set the access time for a file, and expect no
+# other processes to change this during the test run. This is not always the
+# case, so we have a way to disable these tests on systems where the access
+# might not be preserved.
+def _has_consistent_atime(self):
+# NetBSD: noatime mounts currently inhibit 'touch -a' updates.
+if platform.system() == 'NetBSD':
+return False
+
+# Windows: the last access time is disabled by default in the OS, and
+# the check below is written in terms of unix utilities (touch, ls),
+# which will not work on this platform.
+if platform.system() == 'Windows':
+return False
+
+# Other Platforms: Try to use `touch -a` to set the atime, and then to
+# read it with `ls`. If they don't match, or either process errors, then
+# don't claim that atime is consistent.
+with tempfile.NamedTemporaryFile() as f:
+# Specific date in the past on purpose, based on what is attempted
+# in the tests that do the same thing.
+(_, try_touch_err) = self.get_process_output(["touch", "-a", "-t", "199505050555.55", f.name])
+if try_touch_err != "":
+return False
+
+(touch_res_out, touch_res_err) = self.get_process_output(["ls", "-lu", f.name])
+if touch_res_err != "":
+return False
+if "1995" not in touch_res_out:
+return False
+
+return True
+
 def _find_git_windows_unix_tools(self, tools_needed):
 assert(sys.platform == 'win32')
 if sys.version_info.major >= 3:
Index: llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
===
--- llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
+++ llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
@@ -1,7 +1,5 @@
 # Note: ls -lu prints the accessed timestamp
-# NetBSD: noatime mounts currently inhibit 'touch -a' updates
-# Windows: the last access time is disabled by default in the OS
-# UNSUPPORTED: system-netbsd, system-windows
+# REQUIRES: consistent-atime
 
 # Preserve dates when stripping to an output file.
 # RUN: yaml2obj %s -o %t.1.o
Index: llvm/test/ThinLTO/X86/cache.ll
===
--- llvm/test/ThinLTO/X86/cache.ll
+++ llvm/test/ThinLTO/X86/cache.ll
@@ -1,5 +1,4 @@
-; NetBSD: noatime mounts currently inhibit 'touch -a' updates
-; UNSUPPORTED: system-netbsd
+; REQUIRES: consistent-atime
 
 ; The .noindex suffix for output dir is to prevent Spotlight on macOS from
 ; indexing it.
Index: lld/test/wasm/lto/cache.ll
===
--- lld/test/wasm/lto/cache.ll
+++ lld/test/wasm/lto/cache.ll
@@ -1,8 +1,6 @@
 ; RUN: opt -module-hash -module-summary %s -o %t.o
 ; RUN: opt -module-hash -module-summary %p/Inputs/cache.ll -o %t2.o
-; NetBSD: noatime mounts currently inhibit 'touch' from updating atime
-; Windows: no 'touch' command.
-; UNSUPPORTED: system-netbsd, system-windows
+; REQUIRES: consistent-atime
 
 ; RUN: rm -Rf %t.cache && mkdir %t.cache
 ; Create two files that would be removed by cache pruning due to age.
Index: lld/test/MachO/lto-cache.ll
===
--- lld/test/MachO/lto-cache.ll
+++ lld/test/MachO/lto-cache.ll
@@ -1,6 +1,5 @@
 ; REQUIRES: x86
-; NetBSD: noatime mounts currently inhibit 'touch' from updating atime
-; UNSUPPORTED: system-netbsd
+; REQUIRES: consistent-atime
 
 ; RUN: rm -rf %t; split-file %s %t
 ; RUN: opt -module-hash -module-summary %t/foo.ll -o %t/foo.o
Index: 

[PATCH] D144638: [lit] Detect Consistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/utils/lit/lit/llvm/config.py:171
+# in the tests that do the same thing.
+(_, try_touch_err) = self.get_process_output(["touch", "-a", "-t", 
"199505050555.55", f.name])
+if try_touch_err != "":

michaelplatings wrote:
> lenary wrote:
> > michaelplatings wrote:
> > > It looks like this command will be run on Windows. I think it will fail 
> > > and cause False to be returned, which is the desired result, but this 
> > > appears to be by accident rather than design. Therefore I'm inclined to 
> > > agree with @int3 that a hard-coded check would be preferable.
> > I am going to add the hardcoded checks, but I think `touch` is available in 
> > windows, it should be in the same directory as all the git binaries.
> This is definitely tangential to the change, but in case it's useful to know: 
> conventionally only `C:\Program Files\Git\bin` is added to the path on 
> Windows, not `C:\Program Files\Git\usr\bin`. `C:\Program Files\Git\bin` only 
> contains `bash.exe`, `git.exe` and `sh.exe`.
Something weirder is happening in `_find_git_windows_unix_tools` (from 
https://reviews.llvm.org/D84380), but I think it's probably right just to early 
exit with false on Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

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

[PATCH] D144638: [lit] Detect Consistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Updates incoming to add a specific check for netbsd and windows, which are 
currently excluding the affected tests anyway.




Comment at: llvm/utils/lit/lit/llvm/config.py:165
+#
+# This check hopefully detects both cases, and disables tests that require
+# consistent atime.

int3 wrote:
> lenary wrote:
> > jhenderson wrote:
> > > Is "hopefully" really needed here?
> > I'm hedging in this comment, in part because we're trying to find a race 
> > condition experimentally, and also because this will cause lit to fatal 
> > error before running any tests if `touch` exits non-zero for any reason. 
> if we know for sure that NetBSD and Windows don't support this, why not 
> hardcode those values in, and only try the experimental check otherwise?
Yeah, this seems pragmatic, I will add it back in again - excluding NetBSD and 
Windows.



Comment at: llvm/utils/lit/lit/llvm/config.py:171
+# in the tests that do the same thing.
+(_, try_touch_err) = self.get_process_output(["touch", "-a", "-t", 
"199505050555.55", f.name])
+if try_touch_err != "":

michaelplatings wrote:
> It looks like this command will be run on Windows. I think it will fail and 
> cause False to be returned, which is the desired result, but this appears to 
> be by accident rather than design. Therefore I'm inclined to agree with @int3 
> that a hard-coded check would be preferable.
I am going to add the hardcoded checks, but I think `touch` is available in 
windows, it should be in the same directory as all the git binaries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

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

[PATCH] D144638: [lit] Detect Consistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D144638#4147133 , @jhenderson 
wrote:

> This looks reasonable to me, with the caveat that I don't know a huge amount 
> about how the different OSes access time systems work. One question though: 
> if your antivirus was causing flakiness (as opposed to outright 
> always-fails), won't it just move that flakiness into whether the REQUIRES 
> calculation returns true or not (i.e. it could spuriously do so, causing the 
> tests to be enabled but then potential still be flaky?

I'm definitely not an expert in OS atimes either, beyond the comments that have 
accumulated in the repo so far, and knowing that some filesystems prefer to 
have it switched off for performance reasons.

The flakiness I (and colleagues) have seen seems to correspond to computer 
load. In my case, I think the underlying issue is a race between the `touch -a 
-t  ` (which in these test creates ``), then the `ls -ul 
`, and the antivirus, which is watching for file creation, and will also 
access the file. This is also one reason why I put "hopefully" in the comment - 
we're hoping to get the same race during lit configuration. One reason I think 
we're more likely to is that this code is run before we spin up lots of 
parallel threads to start testing, so the test machine should be under less 
load for this check, which should allow the antivirus to get in before the `ls 
-ul`, if all goes to plan (it's a race, so this is hard to be sure about).

I can see your point about this moving the flakiness, but I think that this 
check is more likely to fail in the other direction: marking the tests as 
unsupported when they might be ok. In that case, we lose coverage when we 
didn't need to.




Comment at: llvm/utils/lit/lit/llvm/config.py:165
+#
+# This check hopefully detects both cases, and disables tests that require
+# consistent atime.

jhenderson wrote:
> Is "hopefully" really needed here?
I'm hedging in this comment, in part because we're trying to find a race 
condition experimentally, and also because this will cause lit to fatal error 
before running any tests if `touch` exits non-zero for any reason. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

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

[PATCH] D144638: [lit] Detect Consistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

lenary added reviewers: simonwallis2, mgorny.
lenary added a subscriber: mgorny.
lenary added a comment.

Adding more reviewers. @mgorny you disabled some tests on NetBSD in the past 
for the same reason (rG92dc7dce4a6f117a497ced1650bc48e5b658f0ea 
), this 
just updates how they were disabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

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

[PATCH] D144638: [lit] Detect Consistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
Herald added subscribers: pmatos, asb, ormris, steven_wu, delcypher, hiraditya, 
sbc100, emaste.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: jhenderson.
Herald added projects: lld-macho, All.
Herald added a reviewer: lld-macho.
lenary requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay, aheejin.
Herald added projects: clang, LLVM.

I was seeing flaky tests in my builds due to another process (in my
case, antivirus) modifying the access time of files during the tests,
so those tests using `touch -a` were failing.

This change modifies how we check if a system has consistent atime.
Instead of a list of platforms where we know things might not work, we
instead perform a check similar to the ones used in the tests, but on a
temporary file. If the check is successful, then you get a
`consistent-atime` feature which you can make your test REQUIRE.

This updates all existing tests which mention atime or use `touch -a` to
`REQUIRE: consistent-atime`, fixing their flakiness.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144638

Files:
  clang/test/Modules/prune.m
  lld/test/COFF/lto-cache.ll
  lld/test/ELF/lto/cache.ll
  lld/test/MachO/lto-cache.ll
  lld/test/wasm/lto/cache.ll
  llvm/test/ThinLTO/X86/cache.ll
  llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -5,6 +5,7 @@
 import subprocess
 import sys
 import errno
+import tempfile
 
 import lit.util
 from lit.llvm.subst import FindTool
@@ -155,6 +156,30 @@
 self.with_environment(
 'DYLD_INSERT_LIBRARIES', gmalloc_path_str)
 
+if self._has_consistent_atime():
+features.add('consistent-atime')
+
+# - NetBSD: noatime mounts currently inhibit 'touch -a' updates.
+# - Windows: the last access time is disabled by default in the OS.
+#
+# This check hopefully detects both cases, and disables tests that require
+# consistent atime.
+def _has_consistent_atime(self):
+with tempfile.NamedTemporaryFile() as f:
+# Specific date in the past on purpose, based on what is attempted
+# in the tests that do the same thing.
+(_, try_touch_err) = self.get_process_output(["touch", "-a", "-t", "199505050555.55", f.name])
+if try_touch_err != "":
+return False
+
+(touch_res_out, touch_res_err) = self.get_process_output(["ls", "-lu", f.name])
+if touch_res_err != "":
+return False
+if "1995" not in touch_res_out:
+return False
+
+return True
+
 def _find_git_windows_unix_tools(self, tools_needed):
 assert(sys.platform == 'win32')
 if sys.version_info.major >= 3:
Index: llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
===
--- llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
+++ llvm/test/tools/llvm-objcopy/ELF/strip-preserve-atime.test
@@ -1,7 +1,5 @@
 # Note: ls -lu prints the accessed timestamp
-# NetBSD: noatime mounts currently inhibit 'touch -a' updates
-# Windows: the last access time is disabled by default in the OS
-# UNSUPPORTED: system-netbsd, system-windows
+# REQUIRES: consistent-atime
 
 # Preserve dates when stripping to an output file.
 # RUN: yaml2obj %s -o %t.1.o
Index: llvm/test/ThinLTO/X86/cache.ll
===
--- llvm/test/ThinLTO/X86/cache.ll
+++ llvm/test/ThinLTO/X86/cache.ll
@@ -1,5 +1,4 @@
-; NetBSD: noatime mounts currently inhibit 'touch -a' updates
-; UNSUPPORTED: system-netbsd
+; REQUIRES: consistent-atime
 
 ; The .noindex suffix for output dir is to prevent Spotlight on macOS from
 ; indexing it.
Index: lld/test/wasm/lto/cache.ll
===
--- lld/test/wasm/lto/cache.ll
+++ lld/test/wasm/lto/cache.ll
@@ -1,8 +1,6 @@
 ; RUN: opt -module-hash -module-summary %s -o %t.o
 ; RUN: opt -module-hash -module-summary %p/Inputs/cache.ll -o %t2.o
-; NetBSD: noatime mounts currently inhibit 'touch' from updating atime
-; Windows: no 'touch' command.
-; UNSUPPORTED: system-netbsd, system-windows
+; REQUIRES: consistent-atime
 
 ; RUN: rm -Rf %t.cache && mkdir %t.cache
 ; Create two files that would be removed by cache pruning due to age.
Index: lld/test/MachO/lto-cache.ll
===
--- lld/test/MachO/lto-cache.ll
+++ lld/test/MachO/lto-cache.ll
@@ -1,6 +1,5 @@
 ; REQUIRES: x86
-; NetBSD: noatime mounts currently inhibit 'touch' from updating atime
-; UNSUPPORTED: system-netbsd
+; REQUIRES: consistent-atime
 
 ; RUN: rm 

[PATCH] D140999: [NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h

[Sam Elliott via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c712296fb75: [NFC][TargetParser] Remove 
llvm/Support/AArch64TargetParser.h (authored by lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140999

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

Index: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
===
--- llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -48,7 +48,6 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/SMLoc.h"
-#include "llvm/Support/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/AArch64TargetParser.h"
Index: llvm/lib/Target/AArch64/AArch64Subtarget.cpp
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.cpp
+++ llvm/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -24,8 +24,8 @@
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineScheduler.h"
 #include "llvm/IR/GlobalValue.h"
-#include "llvm/Support/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 
 using namespace llvm;
 
Index: llvm/include/llvm/Support/AArch64TargetParser.h
===
--- llvm/include/llvm/Support/AArch64TargetParser.h
+++ /dev/null
@@ -1,15 +0,0 @@
-//===-- llvm/Support/AArch64TargetParser.h --*- C++ -*-===//
-//
-// 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
-//
-//===--===//
-///
-/// \file
-/// This header is deprecated in favour of
-/// `llvm/TargetParser/AArch64TargetParser.h`.
-///
-//===--===//
-
-#include "llvm/TargetParser/AArch64TargetParser.h"
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -23,10 +23,10 @@
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/MC/TargetRegistry.h"
-#include "llvm/Support/AArch64TargetParser.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 
 #include "lldb/Core/Address.h"
 #include "lldb/Core/Module.h"
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -12,9 +12,9 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
-#include "llvm/Support/AArch64TargetParser.h"
-#include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -14,11 +14,11 @@
 #include "clang/Driver/ToolChain.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/Support/AArch64TargetParser.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SpecialCaseList.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h"
 #include 
 
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -52,10 +52,10 @@
 #include "llvm/IR/IntrinsicsX86.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/MatrixBuilder.h"
-#include "llvm/Support/AArch64TargetParser.h"
 #include 

[PATCH] D140999: [NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Most recent diff was to clang-format the patch, which has removed some 
duplicate includes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140999

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

[PATCH] D140999: [NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 494600.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140999

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

Index: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
===
--- llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -48,7 +48,6 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/SMLoc.h"
-#include "llvm/Support/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/AArch64TargetParser.h"
Index: llvm/lib/Target/AArch64/AArch64Subtarget.cpp
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.cpp
+++ llvm/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -24,8 +24,8 @@
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineScheduler.h"
 #include "llvm/IR/GlobalValue.h"
-#include "llvm/Support/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 
 using namespace llvm;
 
Index: llvm/include/llvm/Support/AArch64TargetParser.h
===
--- llvm/include/llvm/Support/AArch64TargetParser.h
+++ /dev/null
@@ -1,15 +0,0 @@
-//===-- llvm/Support/AArch64TargetParser.h --*- C++ -*-===//
-//
-// 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
-//
-//===--===//
-///
-/// \file
-/// This header is deprecated in favour of
-/// `llvm/TargetParser/AArch64TargetParser.h`.
-///
-//===--===//
-
-#include "llvm/TargetParser/AArch64TargetParser.h"
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -23,10 +23,10 @@
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/MC/TargetRegistry.h"
-#include "llvm/Support/AArch64TargetParser.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 
 #include "lldb/Core/Address.h"
 #include "lldb/Core/Module.h"
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -12,9 +12,9 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
-#include "llvm/Support/AArch64TargetParser.h"
-#include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -14,11 +14,11 @@
 #include "clang/Driver/ToolChain.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/Support/AArch64TargetParser.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SpecialCaseList.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h"
 #include 
 
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -52,10 +52,10 @@
 #include "llvm/IR/IntrinsicsX86.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/MatrixBuilder.h"
-#include "llvm/Support/AArch64TargetParser.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/X86TargetParser.h"
+#include 

[PATCH] D140999: [NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h

[Sam Elliott via Phabricator via cfe-commits]

lenary retitled this revision from "[NFC][TargetParser] Deprecate 
llvm/Support/AArch64TargetParser.h" to "[NFC][TargetParser] Remove 
llvm/Support/AArch64TargetParser.h".
lenary edited the summary of this revision.
lenary updated this revision to Diff 494596.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140999

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

Index: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
===
--- llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -48,7 +48,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/SMLoc.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/AArch64TargetParser.h"
Index: llvm/lib/Target/AArch64/AArch64Subtarget.cpp
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.cpp
+++ llvm/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -24,7 +24,7 @@
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineScheduler.h"
 #include "llvm/IR/GlobalValue.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 
 using namespace llvm;
Index: llvm/include/llvm/Support/AArch64TargetParser.h
===
--- llvm/include/llvm/Support/AArch64TargetParser.h
+++ /dev/null
@@ -1,15 +0,0 @@
-//===-- llvm/Support/AArch64TargetParser.h --*- C++ -*-===//
-//
-// 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
-//
-//===--===//
-///
-/// \file
-/// This header is deprecated in favour of
-/// `llvm/TargetParser/AArch64TargetParser.h`.
-///
-//===--===//
-
-#include "llvm/TargetParser/AArch64TargetParser.h"
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -23,7 +23,7 @@
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/MC/TargetRegistry.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetSelect.h"
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -12,7 +12,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Host.h"
 
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -14,7 +14,7 @@
 #include "clang/Driver/ToolChain.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SpecialCaseList.h"
 #include "llvm/Support/TargetParser.h"
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -52,7 +52,7 @@
 #include "llvm/IR/IntrinsicsX86.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/MatrixBuilder.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/X86TargetParser.h"
Index: 

[PATCH] D140999: [NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D140999#4030131 , @MaskRay wrote:

> I don't think it is necessary to deprecate the old header then delete it 
> after 16.0.0 is branched.
> llvm/Support/AArch64TargetParser.h has very few open-source out-of-tree uses. 
> Perhaps only ldc `driver/targetmachine.cpp` uses the header. So it is not 
> worth extra expedience.
> Just deleting it in another change should be fine.
>
> Changing the include to `#include "llvm/TargetParser/AArch64TargetParser.h"` 
> is totally fine, though.

Sorry, busy time on downstream work means I haven't got back to this.

My plan now is:

- Rebase this to not care about the modulemap patch it is based on, as that is 
getting no review at all, despite being pinged.
- Just start deleting the forwarding headers, as [NFC] commits, updating all 
the references to the new headers. We're after the 16 branch point, so I think 
that's been a long enough cut-over period.

At some point, people will need to fix the bazel/modulemap builds, but I've 
tried the latter and got no traction on patches to do so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140999

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

[PATCH] D139182: AArch64: add CodeGen support for FEAT_XS DSB instructions

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I'm not sure I agree with "map naturally", when `CRm` is a 4-bit field, and 
both bit 7 and bit 5 are different in the `dsb ` vs `dsb nXS` 
instructions.

Are you willing to put this through review for the ACLE 
? 
`__builtin_arm_dsb` is the implementation of `__dsb`, so I worry that this 
would make this intrinsic non-portable for clang vs gcc.


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

https://reviews.llvm.org/D139182

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

[PATCH] D142410: [AArch64] ARMv8.5-A implies both FEAT_SB and FEAT_SSBS

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D142410#4076389 , @philipp.tomsich 
wrote:

> When looking at "ARM DDI 0487G.a", on page A2-68 under the heading 
> "Additional functionality added to Armv8.0 in later releases" in the 
> definition-list item "FEAT_SSBS, Speculative Store Bypass Safe":
>
>> This feature is OPTIONAL in Armv8.0 implementations and mandatory in Armv8.5 
>> implementations.
>
> There is a to FEAT_SBSS in the backlink in "A2.8 The Armv8.5 architecture 
> extension", under the subsection "A2.8.3 Features added to earlier 
> extensions", in the bullet-list labeled "The features that have been added to 
> earlier architectural extensions are:"

I cited version `I.a`, which is later than version `G.a`. Looking at the errata 
for version `G.b`, erratum R18653 (page 93) is where it was relaxed to not be 
mandatory from v8.5a. Errata document link: 
https://developer.arm.com/documentation/102105/gb-05/?lang=en (I'm just happy 
this was in the first errata document I looked for!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142410

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

[PATCH] D142410: [AArch64] ARMv8.5-A implies both FEAT_SB and FEAT_SSBS

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

SSBS is not mandatory from v8.5a onwards. More details/citation inline.

NB: This will need a rebase, because @dmgreen changed how crypto is handled in 
these bitmaps yesterday.




Comment at: llvm/include/llvm/TargetParser/AArch64TargetParser.h:311
 inline constexpr ArchInfo ARMV8_4A  = { VersionTuple{8, 4}, AProfile, 
"armv8.4-a", "+v8.4a", (ARMV8_3A.DefaultExts | AArch64::AEK_DOTPROD)};
-inline constexpr ArchInfo ARMV8_5A  = { VersionTuple{8, 5}, AProfile, 
"armv8.5-a", "+v8.5a", (ARMV8_4A.DefaultExts)};
+inline constexpr ArchInfo ARMV8_5A  = { VersionTuple{8, 5}, AProfile, 
"armv8.5-a", "+v8.5a", (ARMV8_4A.DefaultExts | AArch64::AEK_SB | 
AArch64::AEK_SSBS )};
 constexpr unsigned BaseNoCrypto = ARMV8_5A.DefaultExts ^ AArch64::AEK_CRYPTO; 
// 8.6 onwards has no AEK_CRYPTO

According to the A-profile Arm ARM (version I.a):
- FEAT_SB is mandatory from v8.5 onwards, so thank you for correcting this 
oversight.
- There is no language about the same for FEAT_SSBS/FEAT_SSBS2. It was added to 
v8.0a by v8.5a, but it doesn't seem to have been made mandatory in v8.5a, as 
far as I can see. There is no language in section A2 (Architectural 
Extensions), or in the description of the `ID_AA64PFR1_EL1.SSBS` register about 
being mandatory from certain versions (other fields in this register do have 
this language).



Comment at: llvm/include/llvm/TargetParser/AArch64TargetParser.h:323
 // For v8-R, we do not enable crypto and align with GCC that enables a more 
minimal set of optional architecture extensions.
 inline constexpr ArchInfo ARMV8R= { VersionTuple{8, 0}, RProfile, 
"armv8-r", "+v8r", ((BaseNoCrypto ^ AArch64::AEK_LSE) | AArch64::AEK_SSBS | 
AArch64::AEK_FP16 | AArch64::AEK_FP16FML | AArch64::AEK_SB), };
 // clang-format on

This bitmap is based on the v8.5a bitmap, so would need checking, but it looks 
like it will be correct already because v8r has SB and SSBS marked already 
(which is correct).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142410

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

[PATCH] D140222: [AArch64] Check 128-bit Sysreg Builtins

[Sam Elliott 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 rG03ef89f8d909: [AArch64] Check 128-bit Sysreg Builtins 
(authored by lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140222

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/Headers/arm_acle.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-sysregs-target.c
  clang/test/Sema/aarch64-special-register.c

Index: clang/test/Sema/aarch64-special-register.c
===
--- clang/test/Sema/aarch64-special-register.c
+++ clang/test/Sema/aarch64-special-register.c
@@ -16,6 +16,10 @@
   __builtin_arm_wsr64("sysreg", v);
 }
 
+void wsr128_1(__uint128_t v) {
+  __builtin_arm_wsr128("sysreg", v);
+}
+
 unsigned rsr_1(void) {
   return __builtin_arm_rsr("sysreg");
 }
@@ -28,6 +32,10 @@
   return __builtin_arm_rsr64("sysreg");
 }
 
+__uint128_t rsr128_1(void) {
+  return __builtin_arm_rsr128("sysreg");
+}
+
 void wsr_2(unsigned v) {
   __builtin_arm_wsr("0:1:2:3:4", v);
 }
@@ -52,6 +60,10 @@
   return __builtin_arm_rsr64("0:1:15:15:4");
 }
 
+__uint128_t rsr128_2(void) {
+  return __builtin_arm_rsr128("0:1:15:15:4");
+}
+
 void wsr_3(unsigned v) {
   __builtin_arm_wsr("0:1:2", v); //expected-error {{invalid special register for builtin}}
 }
@@ -64,6 +76,10 @@
   __builtin_arm_wsr64("0:1:2", v); //expected-error {{invalid special register for builtin}}
 }
 
+void wsr128_3(__uint128_t v) {
+  __builtin_arm_wsr128("0:1:2", v); //expected-error {{invalid special register for builtin}}
+}
+
 unsigned rsr_3(void) {
   return __builtin_arm_rsr("0:1:2"); //expected-error {{invalid special register for builtin}}
 }
@@ -99,3 +115,101 @@
 unsigned long rsr64_6(void) {
   return __builtin_arm_rsr64("0:1:16:16:2"); //expected-error {{invalid special register for builtin}}
 }
+
+__uint128_t rsr128_3(void) {
+  return __builtin_arm_rsr128("0:1:2"); //expected-error {{invalid special register for builtin}}
+}
+
+__uint128_t rsr128_4(void) {
+  return __builtin_arm_rsr128("0:1:2:3:8"); //expected-error {{invalid special register for builtin}}
+}
+
+__uint128_t rsr128_5(void) {
+  return __builtin_arm_rsr128("0:8:2:3:4"); //expected-error {{invalid special register for builtin}}
+}
+
+__uint128_t rsr128_6(void) {
+  return __builtin_arm_rsr128("0:1:16:16:2"); //expected-error {{invalid special register for builtin}}
+}
+
+void wsr_4(void) {
+  __builtin_arm_wsr("spsel", 15);
+  __builtin_arm_wsr("daifclr", 15);
+  __builtin_arm_wsr("daifset", 15);
+  __builtin_arm_wsr("pan", 15);
+  __builtin_arm_wsr("uao", 15);
+  __builtin_arm_wsr("dit", 15);
+  __builtin_arm_wsr("ssbs", 15);
+  __builtin_arm_wsr("tco", 15);
+
+  __builtin_arm_wsr("allint", 1);
+  __builtin_arm_wsr("pm", 1);
+}
+
+void wsr64_4(void) {
+  __builtin_arm_wsr("spsel", 15);
+  __builtin_arm_wsr("daifclr", 15);
+  __builtin_arm_wsr("daifset", 15);
+  __builtin_arm_wsr("pan", 15);
+  __builtin_arm_wsr("uao", 15);
+  __builtin_arm_wsr("dit", 15);
+  __builtin_arm_wsr("ssbs", 15);
+  __builtin_arm_wsr("tco", 15);
+
+  __builtin_arm_wsr("allint", 1);
+  __builtin_arm_wsr("pm", 1);
+}
+
+void wsr_5(unsigned v) {
+  __builtin_arm_wsr("spsel", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("daifclr", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("daifset", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("pan", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("uao", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("dit", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("ssbs", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("tco", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("allint", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("pm", v); // expected-error {{must be a constant integer}}
+}
+
+void wsr64_5(unsigned long v) {
+  __builtin_arm_wsr64("spsel", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("daifclr", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("daifset", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("pan", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("uao", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("dit", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("ssbs", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("tco", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("allint", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("pm", v); // expected-error 

[PATCH] D140222: [AArch64] Check 128-bit Sysreg Builtins

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:8297
  << Arg->getSourceRange();
   } else if (IsAArch64Builtin && Fields.size() == 1) {
+// If this is a write ...

tmatheson wrote:
> It might be more readable to outline this whole branch and remove the 
> redundant "else".
Sorry, this is not done, but I'm not sure that fundamentally helps this very 
long function doing too many different things be more readable. I'm going to 
land this today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140222

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

[PATCH] D140959: RFC: Multilib prototype

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Thanks for putting together this proposal.

Broadly, I think it's good, and I think it solves a lot of rough edges that I 
recall running into with the GCC multilib configuration files in the past.

I do worry about using `-cc1` args. On the one hand, it's the best place for 
getting access to information about e.g. exceptions, floating point abi, and 
other concepts (optimisation level?), but on the other hand, it is a reasonably 
unstable interface, and it's parsed in a complex and stateful way -- later 
options can override earlier ones -- which makes it difficult to parse with 
regular expressions.

To be clear: I don't think there's a better way of handling 
exceptions/no-exceptions library selection (than parsing the `-cc1` arguments) 
or the floating point abi library selection, so having a way to examining the 
`-cc1` arguments is a necessary "lowest common denominator" interface (I'll 
come back to this later), but I think there are places we can do better.

In terms of finding a more stable interface, the Arm ACLE recently added Beta 
support for Function Multi-Versioning 

 which is proposing a lot of stable identifiers for architectural features in 
v8-a and v8-r onwards. These names are expected to be written by programmers in 
their source code, so are a stable interface in a way that `-target-feature 
+` is absolutely not. These are resolved by clang in order to handle 
duplicating the same function to compile it for multiple targets, which to my 
mind is the exact same problem that multi-libs are trying to solve. We have 
specified stable names for a lot of features in the ACLE (and presumably will 
continue to do so), as well as a priority system for resolving how to choose 
which function should be used at runtime. I concede that these stable names do 
not cover M-profile or pre-v8 architectures, which are important to embedded 
toolchains.

The information about these stable feature names is known about by 
`AArch64TargetInfo` right now, and we are looking at where exactly this 
information should be best understood by LLVM in general (maybe the 
`TargetParser` library). Given where it is in clang's code, I think it should 
be usable by both the driver and internally inside `-cc1`, but I'm not 100% 
sure the C++ interface is perfect for both. But, we can always revisit the C++ 
interface to make it work for what we want.

So, to concretely turn this into a proposal, I think there are two things you 
could do:

- in the `arguments:` part of the yaml structure, you currently have `regex:` 
which is for matching on the `-cc1` arguments, we could have a `archFeatures:` 
for matching on a list/set of named features (instead of the `regex:`), to turn 
them into multilib library attributes.
- treat the stable architectural feature names like extra implicit attributes 
that don't show up in the `arguments:` section, but can be used in the 
`variants:` section like the `-cc1`-derived attributes. Here, we would need to 
be careful the architectural feature names didn't clash with the user-provided 
names from the `arguments:` section, and we might need to have a way to specify 
the absence of architectural features as well. I'm not 100% convinced that the 
implicit-ness of this option is good, but I think the stability wins are useful.

To cover some drawbacks of the approach of using these stable names:

- Not all targets support function multi-versioning. In this case, they are 
welcome to fall back to using `-cc1` arguments, as a lowest common denominator 
approach.
- Not all multilibs are chosen based on architectural features: again, you can 
fall-back to `-cc1` arguments, especially as I think e.g. `-fno-exceptions` is 
more stable than `-target-feature +`.
- Even for Arm/AArch64, there are pre-v8-a/v8-r architectures which don't have 
stable names in the ACLE yet. Again, in the short term we could use `-cc1` 
arguments, before we stabilise architectural names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140959

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

[PATCH] D141411: [AArch64] Make -march and target("arch=..") attributes imply dependent features

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Can we check this logic, especially around adding both AEK_SVE and AEK_SVE2 in 
the `AARCH64_ARCH` descriptions, this means that `-march=armv9-a+nosve2` still 
can generate sve instructions. I'm not entirely sure of the intended behaviour 
there, especially as sve2 should maybe imply sve by other means (iirc, there's 
a place higher up in AArch64TargetParser.def with the implied features?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141411

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

[PATCH] D140999: [NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h

[Sam Elliott via Phabricator via cfe-commits]

lenary edited the summary of this revision.
lenary updated this revision to Diff 486324.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140999

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

Index: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
===
--- llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -48,7 +48,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/SMLoc.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
Index: llvm/lib/Target/AArch64/AArch64Subtarget.cpp
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.cpp
+++ llvm/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -24,7 +24,7 @@
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineScheduler.h"
 #include "llvm/IR/GlobalValue.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 
 using namespace llvm;
Index: llvm/include/llvm/module.modulemap
===
--- llvm/include/llvm/module.modulemap
+++ llvm/include/llvm/module.modulemap
@@ -454,7 +454,6 @@
 
   // Forwarding Headers, will be removed.
   header "ADT/Triple.h"
-  header "Support/AArch64TargetParser.h"
   header "Support/ARMTargetParserCommon.h"
   header "Support/ARMTargetParser.h"
   header "Support/CSKYTargetParser.h"
Index: llvm/include/llvm/Support/AArch64TargetParser.h
===
--- llvm/include/llvm/Support/AArch64TargetParser.h
+++ llvm/include/llvm/Support/AArch64TargetParser.h
@@ -12,4 +12,5 @@
 ///
 //===--===//
 
+#pragma GCC warning ("This header is deprecated, please use llvm/TargetParser/AArch64TargetParser.h")
 #include "llvm/TargetParser/AArch64TargetParser.h"
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -23,7 +23,7 @@
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/MC/TargetRegistry.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetSelect.h"
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -12,7 +12,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Host.h"
 
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -14,7 +14,7 @@
 #include "clang/Driver/ToolChain.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SpecialCaseList.h"
 #include "llvm/Support/TargetParser.h"
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -52,7 +52,7 @@
 #include "llvm/IR/IntrinsicsX86.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/MatrixBuilder.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/X86TargetParser.h"
Index: clang/lib/Basic/Targets/AArch64.h

[PATCH] D140999: [NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h

[Sam Elliott via Phabricator via cfe-commits]

lenary added reviewers: tmatheson, pratlucas, MaskRay.
lenary added a comment.
Herald added subscribers: StephenFan, JDevlieghere.

Adding some relevant reviewers.

My plan is to land the other patches like this directly onto `main`, as they 
are NFC and a cleanup. I hope this is uncontroversial, but let me know if you 
think it wouldn't be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140999

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

[PATCH] D140999: [NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
lenary requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLDB, LLVM.

This deprecates, but does not remove, `llvm/Support/AArch64TargetParser.h`.

I am proposing to do this for all the forwarding headers left after
rGf09cf34d00625e57dea5317a3ac0412c07292148 
 - for 
each header:

- Update all relevant in-tree includes
- Add a `#pragma GCC warning ...` to note the header is deprecated.
- Leave the header in place in both the source and the modulemap until after 
LLVM 16 is branched.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140999

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

Index: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
===
--- llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -48,7 +48,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/SMLoc.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
Index: llvm/lib/Target/AArch64/AArch64Subtarget.cpp
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.cpp
+++ llvm/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -24,7 +24,7 @@
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineScheduler.h"
 #include "llvm/IR/GlobalValue.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 
 using namespace llvm;
Index: llvm/include/llvm/Support/AArch64TargetParser.h
===
--- llvm/include/llvm/Support/AArch64TargetParser.h
+++ llvm/include/llvm/Support/AArch64TargetParser.h
@@ -12,4 +12,5 @@
 ///
 //===--===//
 
+#pragma GCC warning ("This header is deprecated, please use llvm/TargetParser/AArch64TargetParser.h")
 #include "llvm/TargetParser/AArch64TargetParser.h"
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -23,7 +23,7 @@
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/MC/TargetRegistry.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetSelect.h"
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -12,7 +12,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Host.h"
 
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -14,7 +14,7 @@
 #include "clang/Driver/ToolChain.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SpecialCaseList.h"
 #include "llvm/Support/TargetParser.h"
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -52,7 +52,7 @@
 #include "llvm/IR/IntrinsicsX86.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/MatrixBuilder.h"
-#include "llvm/Support/AArch64TargetParser.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include 

[PATCH] D123609: [Clang] Remove support for legacy pass manager

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.
Herald added a subscriber: pcwang-thead.

In D123609#3451334 , @nikic wrote:

> In D123609#3451320 , @HaohaiWen 
> wrote:
>
>> Hi @nikic,
>> We recently noticed legacy PM was removed from many places.
>> Does community plan to remove legacy PM completely?
>> Do you know when will CG switch to new PM?
>> Thanks.
>
> At this time, the ability to run a middle-end optimization pipeline using the 
> legacy pass manager (basically anything based on PassManagerBuilder) is being 
> removed. The CodeGen pipeline continues to use the legacy pass manager at 
> this time -- I'm not aware of anyone doing active migration work in that area.

Sorry to resurrect an old patch, but can you please update the docs here 
https://llvm.org/docs/NewPassManager.html#status-of-the-new-and-legacy-pass-managers
 with this info?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123609

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

[PATCH] D137838: [Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137838#4018930 , @vitalybuka 
wrote:

> This bot is broken after the patch 
> https://lab.llvm.org/buildbot/#/builders/236/builds/1480

Thanks for letting me know. I'm back in the office on the 3rd, and it looks 
like this will be top of my list to look at, not that I'm very familiar with 
the pass and what it's up to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137838: [Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137838#4010987 , @dblaikie wrote:

> This has introduced a circular dependency due to the forwarding header 
> (forwarding header depends on new lib, new lib depends on support, where the 
> forwarding header is). Generally this wouldn't be acceptable (& I'd suggest 
> the patch be reverted on that basis) though I understand this is a 
> complicated migration - what's the timeline for correcting this issue?

A commit was landed to add TargetParser to the module map so it looks like part 
of Support, which solves the circular dependency issue in the short term, I 
believe.

https://reviews.llvm.org/D140420 is a proposed fix which should more clearly 
split the TargetParser from Support, but I'm not super familiar with modules 
and my linux dev environment does not work with a modules build. I'd appreciate 
any comments or guidance though I also know it needs an update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19
 
+target_include_directories(LLVMAArch64AsmParser PRIVATE 
${LLVM_LIBRARY_DIR}/TargetParser/)

fpetrogalli wrote:
> lenary wrote:
> > fpetrogalli wrote:
> > > craig.topper wrote:
> > > > fpetrogalli wrote:
> > > > > lenary wrote:
> > > > > > craig.topper wrote:
> > > > > > > Why do we need to touch CMake file that aren't RISC-V?
> > > > > > Yeah, this shouldn't be needed.
> > > > > > 
> > > > > > We do have some fixes for the modules build which recently landed, 
> > > > > > maybe they fix the issues you were seeing, including:
> > > > > > - 
> > > > > > https://reviews.llvm.org/rG9cd6fbee7ed881f8e80b735e95567040e56f189e
> > > > > > - 
> > > > > > https://reviews.llvm.org/rG6bdf378dcd349d97152846bb687c1d1de511d138
> > > > > > - https://reviews.llvm.org/D140420 (this isn't landed, but it might 
> > > > > > clear up some weird things about the quicker modulemap fixes)
> > > > > This is unrelated to Modules. The .inc file generated by tablegen is 
> > > > > created in `{make_build_folder}/lib/TargetParser`. The file is then 
> > > > > included in `TargetParser.cpp` but also in `TargetParser.h` -> this 
> > > > > means that every time we include the latter in a cpp file we need to 
> > > > > make the inc file visible for inclusion.
> > > > Can we split RISC-V out of TargetParser.cpp and TargetParser.h?
> > > Of course! :) I'll do it in a separate patch.
> > I think, if that is the case, we should add 
> > `{make_build_folder}/lib/TargetParser` as a public include directory of the 
> > LLVMTargetParser library, which should mean anything depending on it gets 
> > that as an include directory they can rely on.
> > I think, if that is the case, we should add 
> > `{make_build_folder}/lib/TargetParser` as a public include directory of the 
> > LLVMTargetParser library, which should mean anything depending on it gets 
> > that as an include directory they can rely on.
> 
> How do I do that?I tried `target_include_directories(LLVMTargetParser PUBLIC 
> ${LLVM_LIBRARY_DIR}/TargetParser/)` but I got: 
> ```
> CMake Error in lib/TargetParser/CMakeLists.txt:
>   Target "LLVMTargetParser" INTERFACE_INCLUDE_DIRECTORIES property contains
>   path:
> 
> "/Users/fpetrogalli/<...>/build-shared/./lib/TargetParser/"
> 
>   which is prefixed in the build directory.
> 
> ```
I think https://stackoverflow.com/a/25681179 points to how to do this in such a 
way that it works properly for people using cmake install. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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

[PATCH] D137838: [Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137838#4008605 , @lenary wrote:

> I'm working on a follow-up, which should make the split a bit clearer, but 
> I'm also not a modulemap expert and the `-DLLVM_ENABLE_MODULES=On` 
> configuration is broken on my linux dev box. I'll post it for review if those 
> two patches have at least made the build greener.

Here: https://reviews.llvm.org/D140420 - I'm not sure how urgent this is, if 
your patches did get your builds green. I think in e.g. the bazel build, 
there's still a bunch of places where TargetParser is closely entwined with 
Support that we can fix in the same way we intend to fix the forwarding headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19
 
+target_include_directories(LLVMAArch64AsmParser PRIVATE 
${LLVM_LIBRARY_DIR}/TargetParser/)

fpetrogalli wrote:
> craig.topper wrote:
> > fpetrogalli wrote:
> > > lenary wrote:
> > > > craig.topper wrote:
> > > > > Why do we need to touch CMake file that aren't RISC-V?
> > > > Yeah, this shouldn't be needed.
> > > > 
> > > > We do have some fixes for the modules build which recently landed, 
> > > > maybe they fix the issues you were seeing, including:
> > > > - https://reviews.llvm.org/rG9cd6fbee7ed881f8e80b735e95567040e56f189e
> > > > - https://reviews.llvm.org/rG6bdf378dcd349d97152846bb687c1d1de511d138
> > > > - https://reviews.llvm.org/D140420 (this isn't landed, but it might 
> > > > clear up some weird things about the quicker modulemap fixes)
> > > This is unrelated to Modules. The .inc file generated by tablegen is 
> > > created in `{make_build_folder}/lib/TargetParser`. The file is then 
> > > included in `TargetParser.cpp` but also in `TargetParser.h` -> this means 
> > > that every time we include the latter in a cpp file we need to make the 
> > > inc file visible for inclusion.
> > Can we split RISC-V out of TargetParser.cpp and TargetParser.h?
> Of course! :) I'll do it in a separate patch.
I think, if that is the case, we should add 
`{make_build_folder}/lib/TargetParser` as a public include directory of the 
LLVMTargetParser library, which should mean anything depending on it gets that 
as an include directory they can rely on.



Comment at: llvm/lib/Target/RISCV/CMakeLists.txt:69
+  DEPENDS
+  LLVMTargetParser
   )

Why is this needed? I already added TargetParser to the list of required 
components, on (new) line 61, is this doing something else?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

One comment on the build changes, I don't have opinions on the RISC-V changes.




Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19
 
+target_include_directories(LLVMAArch64AsmParser PRIVATE 
${LLVM_LIBRARY_DIR}/TargetParser/)

craig.topper wrote:
> Why do we need to touch CMake file that aren't RISC-V?
Yeah, this shouldn't be needed.

We do have some fixes for the modules build which recently landed, maybe they 
fix the issues you were seeing, including:
- https://reviews.llvm.org/rG9cd6fbee7ed881f8e80b735e95567040e56f189e
- https://reviews.llvm.org/rG6bdf378dcd349d97152846bb687c1d1de511d138
- https://reviews.llvm.org/D140420 (this isn't landed, but it might clear up 
some weird things about the quicker modulemap fixes)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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

[PATCH] D137838: [Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I'm working on a follow-up, which should make the split a bit clearer, but I'm 
also not a modulemap expert and the `-DLLVM_ENABLE_MODULES=On` configuration is 
broken on my linux dev box. I'll post it for review if those two patches have 
at least made the build greener.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137838: [Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137838#4008443 , @aprantl wrote:

> Can you please update `llvm/include/llvm/module.modulemap` for this change or 
> revert the patch? This is breaking all bots that build with 
> `-DLLVM_ENABLE_MODULES=On`.
>
> For example: 
> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/lastFailedBuild/consoleFull#111065754949ba4694-19c4-4d7e-bec5-911270d8a58c

Looking at this now. Thanks for pointing it out as I didn't get a notification 
for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D140222: [AArch64] Check 128-bit Sysreg Builtins

[Sam Elliott via Phabricator via cfe-commits]

lenary marked 3 inline comments as done.
lenary added a comment.

Nits addressed. Will land in the new year, when I'm back at work. Right now 
it's time for a break!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140222

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

[PATCH] D140222: [AArch64] Check 128-bit Sysreg Builtins

[Sam Elliott via Phabricator via cfe-commits]

lenary edited the summary of this revision.
lenary updated this revision to Diff 484281.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140222

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/Headers/arm_acle.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-sysregs-target.c
  clang/test/Sema/aarch64-special-register.c

Index: clang/test/Sema/aarch64-special-register.c
===
--- clang/test/Sema/aarch64-special-register.c
+++ clang/test/Sema/aarch64-special-register.c
@@ -16,6 +16,10 @@
   __builtin_arm_wsr64("sysreg", v);
 }
 
+void wsr128_1(__uint128_t v) {
+  __builtin_arm_wsr128("sysreg", v);
+}
+
 unsigned rsr_1(void) {
   return __builtin_arm_rsr("sysreg");
 }
@@ -28,6 +32,10 @@
   return __builtin_arm_rsr64("sysreg");
 }
 
+__uint128_t rsr128_1(void) {
+  return __builtin_arm_rsr128("sysreg");
+}
+
 void wsr_2(unsigned v) {
   __builtin_arm_wsr("0:1:2:3:4", v);
 }
@@ -52,6 +60,10 @@
   return __builtin_arm_rsr64("0:1:15:15:4");
 }
 
+__uint128_t rsr128_2(void) {
+  return __builtin_arm_rsr128("0:1:15:15:4");
+}
+
 void wsr_3(unsigned v) {
   __builtin_arm_wsr("0:1:2", v); //expected-error {{invalid special register for builtin}}
 }
@@ -64,6 +76,10 @@
   __builtin_arm_wsr64("0:1:2", v); //expected-error {{invalid special register for builtin}}
 }
 
+void wsr128_3(__uint128_t v) {
+  __builtin_arm_wsr128("0:1:2", v); //expected-error {{invalid special register for builtin}}
+}
+
 unsigned rsr_3(void) {
   return __builtin_arm_rsr("0:1:2"); //expected-error {{invalid special register for builtin}}
 }
@@ -99,3 +115,101 @@
 unsigned long rsr64_6(void) {
   return __builtin_arm_rsr64("0:1:16:16:2"); //expected-error {{invalid special register for builtin}}
 }
+
+__uint128_t rsr128_3(void) {
+  return __builtin_arm_rsr128("0:1:2"); //expected-error {{invalid special register for builtin}}
+}
+
+__uint128_t rsr128_4(void) {
+  return __builtin_arm_rsr128("0:1:2:3:8"); //expected-error {{invalid special register for builtin}}
+}
+
+__uint128_t rsr128_5(void) {
+  return __builtin_arm_rsr128("0:8:2:3:4"); //expected-error {{invalid special register for builtin}}
+}
+
+__uint128_t rsr128_6(void) {
+  return __builtin_arm_rsr128("0:1:16:16:2"); //expected-error {{invalid special register for builtin}}
+}
+
+void wsr_4(void) {
+  __builtin_arm_wsr("spsel", 15);
+  __builtin_arm_wsr("daifclr", 15);
+  __builtin_arm_wsr("daifset", 15);
+  __builtin_arm_wsr("pan", 15);
+  __builtin_arm_wsr("uao", 15);
+  __builtin_arm_wsr("dit", 15);
+  __builtin_arm_wsr("ssbs", 15);
+  __builtin_arm_wsr("tco", 15);
+
+  __builtin_arm_wsr("allint", 1);
+  __builtin_arm_wsr("pm", 1);
+}
+
+void wsr64_4(void) {
+  __builtin_arm_wsr("spsel", 15);
+  __builtin_arm_wsr("daifclr", 15);
+  __builtin_arm_wsr("daifset", 15);
+  __builtin_arm_wsr("pan", 15);
+  __builtin_arm_wsr("uao", 15);
+  __builtin_arm_wsr("dit", 15);
+  __builtin_arm_wsr("ssbs", 15);
+  __builtin_arm_wsr("tco", 15);
+
+  __builtin_arm_wsr("allint", 1);
+  __builtin_arm_wsr("pm", 1);
+}
+
+void wsr_5(unsigned v) {
+  __builtin_arm_wsr("spsel", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("daifclr", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("daifset", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("pan", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("uao", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("dit", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("ssbs", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("tco", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("allint", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("pm", v); // expected-error {{must be a constant integer}}
+}
+
+void wsr64_5(unsigned long v) {
+  __builtin_arm_wsr64("spsel", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("daifclr", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("daifset", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("pan", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("uao", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("dit", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("ssbs", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("tco", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("allint", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr64("pm", v); // expected-error {{must be a constant integer}}
+}
+
+void wsr_6(void) {
+  __builtin_arm_wsr("spsel", 16); // expected-error {{outside the valid range}}
+  

[PATCH] D137838: [Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137838#4007920 , @thakis wrote:

> Thanks for the heads-up. I updated the GN build in 
> 4ac51dd53d93b8dd18c58093766483c657fe3a08 
>  and 
> 2aa998d22fe09191cd6c1b697e373266c1131502 
> . The 
> latter commit has a python script in the commit message that might be useful 
> to others who want to port this to other build systems.

Thanks! Much appreciated!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137838: [Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

And a fixup for the compiler-rt symbolizer: 
https://reviews.llvm.org/rGecaab107e4d0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137838: [Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary marked an inline comment as done.
lenary added a comment.

Another fixup for the llvm examples was required, landed in 
https://reviews.llvm.org/rG16c4c4e04c14


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137838: [Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/ADT/Triple.h:11
+/// This header is deprecated in favour of
+/// `llvm/TargetParser/AArch64TargetParser.h`.
 ///

barannikov88 wrote:
> Invalid comment.
Fixed in https://reviews.llvm.org/rG741396af16d1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137838: [Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

This caused an issue in the libc benchmarks, fix here: 2a261a7b5764 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D140222: [AArch64] Check 128-bit Sysreg Builtins

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
lenary requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch contains several related changes:

1. We move to using TARGET_BUILTIN for the 128-bit system register builtins to 
give better error messages when d128 has not been enabled, or has been enabled 
in a per-function manner.

2. We now validate the inputs to the 128-bit system register builtins, like we 
validate the other system register builtins.

3. We update the list of named PSTATE accessors for MSR (immediate), and now 
correctly enforce the expected ranges of the immediates. There is a long 
comment about how we chose to do this to comply with the ACLE when most of the 
PSTATE accessors for MSR (immediate) have aliased system registers for MRS/MSR 
which expect different values. In short, the MSR (immediate) names are 
prioritised, rather than falling-back to the register form when the value is 
out of range.

Depends on D140221 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140222

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/Headers/arm_acle.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-sysregs-target.c
  clang/test/Sema/aarch64-special-register.c

Index: clang/test/Sema/aarch64-special-register.c
===
--- clang/test/Sema/aarch64-special-register.c
+++ clang/test/Sema/aarch64-special-register.c
@@ -16,6 +16,10 @@
   __builtin_arm_wsr64("sysreg", v);
 }
 
+void wsr128_1(__uint128_t v) {
+  __builtin_arm_wsr128("sysreg", v);
+}
+
 unsigned rsr_1(void) {
   return __builtin_arm_rsr("sysreg");
 }
@@ -28,6 +32,10 @@
   return __builtin_arm_rsr64("sysreg");
 }
 
+__uint128_t rsr128_1(void) {
+  return __builtin_arm_rsr128("sysreg");
+}
+
 void wsr_2(unsigned v) {
   __builtin_arm_wsr("0:1:2:3:4", v);
 }
@@ -52,6 +60,10 @@
   return __builtin_arm_rsr64("0:1:15:15:4");
 }
 
+__uint128_t rsr128_2(void) {
+  return __builtin_arm_rsr128("0:1:15:15:4");
+}
+
 void wsr_3(unsigned v) {
   __builtin_arm_wsr("0:1:2", v); //expected-error {{invalid special register for builtin}}
 }
@@ -64,6 +76,10 @@
   __builtin_arm_wsr64("0:1:2", v); //expected-error {{invalid special register for builtin}}
 }
 
+void wsr128_3(__uint128_t v) {
+  __builtin_arm_wsr128("0:1:2", v); //expected-error {{invalid special register for builtin}}
+}
+
 unsigned rsr_3(void) {
   return __builtin_arm_rsr("0:1:2"); //expected-error {{invalid special register for builtin}}
 }
@@ -99,3 +115,101 @@
 unsigned long rsr64_6(void) {
   return __builtin_arm_rsr64("0:1:16:16:2"); //expected-error {{invalid special register for builtin}}
 }
+
+__uint128_t rsr128_3(void) {
+  return __builtin_arm_rsr128("0:1:2"); //expected-error {{invalid special register for builtin}}
+}
+
+__uint128_t rsr128_4(void) {
+  return __builtin_arm_rsr128("0:1:2:3:8"); //expected-error {{invalid special register for builtin}}
+}
+
+__uint128_t rsr128_5(void) {
+  return __builtin_arm_rsr128("0:8:2:3:4"); //expected-error {{invalid special register for builtin}}
+}
+
+__uint128_t rsr128_6(void) {
+  return __builtin_arm_rsr128("0:1:16:16:2"); //expected-error {{invalid special register for builtin}}
+}
+
+void wsr_4(void) {
+  __builtin_arm_wsr("spsel", 15);
+  __builtin_arm_wsr("daifclr", 15);
+  __builtin_arm_wsr("daifset", 15);
+  __builtin_arm_wsr("pan", 15);
+  __builtin_arm_wsr("uao", 15);
+  __builtin_arm_wsr("dit", 15);
+  __builtin_arm_wsr("ssbs", 15);
+  __builtin_arm_wsr("tco", 15);
+
+  __builtin_arm_wsr("allint", 1);
+  __builtin_arm_wsr("pm", 1);
+}
+
+void wsr64_4(void) {
+  __builtin_arm_wsr("spsel", 15);
+  __builtin_arm_wsr("daifclr", 15);
+  __builtin_arm_wsr("daifset", 15);
+  __builtin_arm_wsr("pan", 15);
+  __builtin_arm_wsr("uao", 15);
+  __builtin_arm_wsr("dit", 15);
+  __builtin_arm_wsr("ssbs", 15);
+  __builtin_arm_wsr("tco", 15);
+
+  __builtin_arm_wsr("allint", 1);
+  __builtin_arm_wsr("pm", 1);
+}
+
+void wsr_5(unsigned v) {
+  __builtin_arm_wsr("spsel", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("daifclr", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("daifset", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("pan", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("uao", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("dit", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("ssbs", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("tco", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("allint", v); // expected-error {{must be a constant integer}}
+  __builtin_arm_wsr("pm", v); // expected-error {{must be a constant integer}}
+}
+
+void wsr64_5(unsigned long v) {
+ 

[PATCH] D139443: [AArch64] Support SLC in ACLE prefetch intrinsics

[Sam Elliott via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
lenary marked 2 inline comments as done.
Closed by commit rG82b51a142804: [AArch64] Support SLC in ACLE prefetch 
intrinsics (authored by lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139443

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/builtins-arm64.c
  clang/test/Sema/builtins-arm64.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
  llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll

Index: llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll
@@ -0,0 +1,67 @@
+; RUN: llc -mtriple=aarch64 -mattr=+v8.9a --global-isel=0 < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64 -mattr=+v8.9a --global-isel=1 --global-isel-abort=1 < %s | FileCheck %s
+
+@a = internal global ptr null, align 8
+@b = external global ptr, align 8
+
+define void @test(ptr %i, i32 %j) nounwind ssp {
+entry:
+  ; CHECK-LABEL: @test
+  %j.addr = alloca i32, align 4
+  store i32 %j, ptr %j.addr, align 4, !tbaa !0
+  %tmp = bitcast ptr %j.addr to ptr
+
+  %i.next = getelementptr i8, ptr %i, i64 2
+
+  ; Verify prefetching works for all the different kinds of pointers we might
+  ; want to prefetch.
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfum pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %tmp, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %i, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfum pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %i.next, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr @a, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr @b, i32 0, i32 0, i32 0, i32 1)
+
+  ; Verify that we can generate every single valid prefetch value.
+
+  ; CHECK: prfm pstl1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 1, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl2keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 1, i32 0, i32 1)
+
+  ; CHECK: prfm pldl3keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 2, i32 0, i32 1)
+
+  ; CHECK: prfm pldslckeep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 3, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1strm,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 1, i32 1)
+
+  ; CHECK: prfm plil1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 0, i32 0)
+
+  ret void
+}
+
+declare void @llvm.aarch64.prefetch(ptr readonly, i32 immarg, i32 immarg, i32 immarg, i32 immarg) #0
+
+attributes #0 = { inaccessiblemem_or_argmemonly nounwind willreturn }
+
+!0 = !{!"int", !1}
+!1 = !{!"omnipotent char", !2}
+!2 = !{!"Simple C/C++ TBAA"}
+!3 = !{!"any pointer", !1}
Index: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
@@ -1070,6 +1070,24 @@
 MI.eraseFromParent();
 return true;
   }
+  case Intrinsic::aarch64_prefetch: {
+MachineIRBuilder MIB(MI);
+auto  = MI.getOperand(1);
+
+int64_t IsWrite = MI.getOperand(2).getImm();
+int64_t Target = MI.getOperand(3).getImm();
+int64_t IsStream = MI.getOperand(4).getImm();
+int64_t IsData = MI.getOperand(5).getImm();
+
+unsigned PrfOp = (IsWrite << 4) |// Load/Store bit
+ (!IsData << 3) |// IsDataCache bit
+ (Target << 1) | // Cache level bits
+ (unsigned)IsStream; // Stream bit
+
+MIB.buildInstr(AArch64::G_PREFETCH).addImm(PrfOp).add(AddrVal);
+MI.eraseFromParent();
+return true;
+  }
   }
 
   return true;
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.h
===
--- llvm/lib/Target/AArch64/AArch64ISelLowering.h
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.h
@@ -960,6 +960,7 @@
 
   SDValue LowerINTRINSIC_W_CHAIN(SDValue Op, SelectionDAG ) const;
   SDValue LowerINTRINSIC_WO_CHAIN(SDValue Op, SelectionDAG ) const;
+  SDValue LowerINTRINSIC_VOID(SDValue Op, SelectionDAG ) const;
 
   bool
   isEligibleForTailCallOptimization(const CallLoweringInfo ) const;
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
===
--- 

[PATCH] D137838: [Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

The most recent update is all the fixes needed after the builds @MaskRay asked 
me for. I think this is ready to land on Monday?

@thakis there will be GN fallout from this change. I do not intend to update GN 
in this patchset, but wanted to give you a heads-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D139443: [AArch64] Support SLC in ACLE prefetch intrinsics

[Sam Elliott via Phabricator via cfe-commits]

lenary added reviewers: paquette, aemerson.
lenary added a comment.

Adding Jessica and Amara as this affects GlobalISel for AArch64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139443

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

[PATCH] D139443: [AArch64] Support SLC in ACLE prefetch intrinsics

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 482489.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139443

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/builtins-arm64.c
  clang/test/Sema/builtins-arm64.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
  llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll

Index: llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll
@@ -0,0 +1,67 @@
+; RUN: llc -mtriple=aarch64 -mattr=+v8.9a --global-isel=0 < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64 -mattr=+v8.9a --global-isel=1 --global-isel-abort=1 < %s | FileCheck %s
+
+@a = internal global ptr null, align 8
+@b = external global ptr, align 8
+
+define void @test(ptr %i, i32 %j) nounwind ssp {
+entry:
+  ; CHECK-LABEL: @test
+  %j.addr = alloca i32, align 4
+  store i32 %j, ptr %j.addr, align 4, !tbaa !0
+  %tmp = bitcast ptr %j.addr to ptr
+
+  %i.next = getelementptr i8, ptr %i, i64 2
+
+  ; Verify prefetching works for all the different kinds of pointers we might
+  ; want to prefetch.
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfum pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %tmp, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %i, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfum pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %i.next, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr @a, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr @b, i32 0, i32 0, i32 0, i32 1)
+
+  ; Verify that we can generate every single valid prefetch value.
+
+  ; CHECK: prfm pstl1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 1, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl2keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 1, i32 0, i32 1)
+
+  ; CHECK: prfm pldl3keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 2, i32 0, i32 1)
+
+  ; CHECK: prfm pldslckeep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 3, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1strm,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 1, i32 1)
+
+  ; CHECK: prfm plil1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 0, i32 0)
+
+  ret void
+}
+
+declare void @llvm.aarch64.prefetch(ptr readonly, i32 immarg, i32 immarg, i32 immarg, i32 immarg) #0
+
+attributes #0 = { inaccessiblemem_or_argmemonly nounwind willreturn }
+
+!0 = !{!"int", !1}
+!1 = !{!"omnipotent char", !2}
+!2 = !{!"Simple C/C++ TBAA"}
+!3 = !{!"any pointer", !1}
Index: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
@@ -1063,6 +1063,24 @@
 MI.eraseFromParent();
 return true;
   }
+  case Intrinsic::aarch64_prefetch: {
+MachineIRBuilder MIB(MI);
+auto  = MI.getOperand(1);
+
+int64_t IsWrite = MI.getOperand(2).getImm();
+int64_t Target = MI.getOperand(3).getImm();
+int64_t IsStream = MI.getOperand(4).getImm();
+int64_t IsData = MI.getOperand(5).getImm();
+
+unsigned PrfOp = (IsWrite << 4) |// Load/Store bit
+ (!IsData << 3) |// IsDataCache bit
+ (Target << 1) | // Cache level bits
+ (unsigned)IsStream; // Stream bit
+
+MIB.buildInstr(AArch64::G_PREFETCH).addImm(PrfOp).add(AddrVal);
+MI.eraseFromParent();
+return true;
+  }
   }
 
   return true;
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.h
===
--- llvm/lib/Target/AArch64/AArch64ISelLowering.h
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.h
@@ -956,6 +956,7 @@
 
   SDValue LowerINTRINSIC_W_CHAIN(SDValue Op, SelectionDAG ) const;
   SDValue LowerINTRINSIC_WO_CHAIN(SDValue Op, SelectionDAG ) const;
+  SDValue LowerINTRINSIC_VOID(SDValue Op, SelectionDAG ) const;
 
   bool
   isEligibleForTailCallOptimization(const CallLoweringInfo ) const;
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
===
--- llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -1189,9 +1189,6 @@
 }
   }
 
-  if (Subtarget->hasSME())
-setOperationAction(ISD::INTRINSIC_VOID, MVT::Other, Custom);
-
   if 

[PATCH] D139443: [AArch64] Support SLC in ACLE prefetch intrinsics

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll:2
+; RUN: llc -mtriple=aarch64 -mattr=+v8.9a < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64 -mattr=+v8.9a -O0 --global-isel-abort=1 < %s | 
FileCheck %s
+

tschuett wrote:
> I believe you also have to turn on GISL: `-global-isel`.
> 
> See test in https://reviews.llvm.org/D109827.
I'm not sure this is necessary, global isel is enabled at O0 on AArch64, and 
e.g. RegBankSelect appears in the output of `llc -mtriple=aarch64 -mattr=+v8.9a 
-O0 < llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll -debug-pass=Structure -o 
arm64-prefetch-new.ll`

Would you find it clearer as:
```
; RUN: llc -mtriple=aarch64 -mattr=+v8.9a --global-isel=0 < %s | FileCheck %s
; RUN: llc -mtriple=aarch64 -mattr=+v8.9a --global-isel=1 --global-isel-abort=1 
< %s | FileCheck %s
```
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139443

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

[PATCH] D139443: [AArch64] Support SLC in ACLE prefetch intrinsics

[Sam Elliott via Phabricator via cfe-commits]

lenary added a reviewer: sdesmalen.
lenary added a subscriber: sdesmalen.
lenary added a comment.

@sdesmalen adding you as I've moved around the custom lowering of 
`INTRINSIC_VOID` to be its own function, and as we need to handle it for 
`@llvm.arm.prefetch` all the time, it's no longer conditional on SME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139443

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

[PATCH] D139443: [AArch64] Support SLC in ACLE prefetch intrinsics

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
lenary requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This change:

- Modifies the ACLE code to allow the new SLC value (3) for the prefetch target.

- Introduces a new intrinsic, @llvm.aarch64.prefetch which matches the PRFM 
family instructions much more closely, and can represent all values for the 
PRFM immediate.

  The target-independent @llvm.prefetch intrinsic does not have enough 
information for us to be able to lower to it from the ACLE intrinsics correctly.

- Lowers the acle calls to the new intrinsic on aarch64 (the ARM lowering is 
unchanged).

- Implements code generation for the new intrinsic in both SelectionDAG and 
GlobalISel. We specifically choose to continue to support lowering the 
target-independent @llvm.prefetch intrinsic so that other frontends can 
continue to use it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139443

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/builtins-arm64.c
  clang/test/Sema/builtins-arm64.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
  llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll

Index: llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll
@@ -0,0 +1,67 @@
+; RUN: llc -mtriple=aarch64 -mattr=+v8.9a < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64 -mattr=+v8.9a -O0 --global-isel-abort=1 < %s | FileCheck %s
+
+@a = internal global ptr null, align 8
+@b = external global ptr, align 8
+
+define void @test(ptr %i, i32 %j) nounwind ssp {
+entry:
+  ; CHECK-LABEL: @test
+  %j.addr = alloca i32, align 4
+  store i32 %j, ptr %j.addr, align 4, !tbaa !0
+  %tmp = bitcast ptr %j.addr to ptr
+
+  %i.next = getelementptr i8, ptr %i, i64 2
+
+  ; Verify prefetching works for all the different kinds of pointers we might
+  ; want to prefetch.
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfum pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %tmp, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %i, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfum pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %i.next, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr @a, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr @b, i32 0, i32 0, i32 0, i32 1)
+
+  ; Verify that we can generate every single valid prefetch value.
+
+  ; CHECK: prfm pstl1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 1, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl2keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 1, i32 0, i32 1)
+
+  ; CHECK: prfm pldl3keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 2, i32 0, i32 1)
+
+  ; CHECK: prfm pldslckeep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 3, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1strm,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 1, i32 1)
+
+  ; CHECK: prfm plil1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 0, i32 0)
+
+  ret void
+}
+
+declare void @llvm.aarch64.prefetch(ptr readonly, i32 immarg, i32 immarg, i32 immarg, i32 immarg) #0
+
+attributes #0 = { inaccessiblemem_or_argmemonly nounwind willreturn }
+
+!0 = !{!"int", !1}
+!1 = !{!"omnipotent char", !2}
+!2 = !{!"Simple C/C++ TBAA"}
+!3 = !{!"any pointer", !1}
Index: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
@@ -1063,6 +1063,24 @@
 MI.eraseFromParent();
 return true;
   }
+  case Intrinsic::aarch64_prefetch: {
+MachineIRBuilder MIB(MI);
+auto  = MI.getOperand(1);
+
+int64_t IsWrite = MI.getOperand(2).getImm();
+int64_t Target = MI.getOperand(3).getImm();
+int64_t IsStream = MI.getOperand(4).getImm();
+int64_t IsData = MI.getOperand(5).getImm();
+
+unsigned PrfOp = (IsWrite << 4) |// Load/Store bit
+ (!IsData << 3) |// IsDataCache bit
+ (Target << 1) | // Cache level bits
+ (unsigned)IsStream; // Stream bit
+
+MIB.buildInstr(AArch64::G_PREFETCH).addImm(PrfOp).add(AddrVal);
+MI.eraseFromParent();
+return true;
+  }
   }
 
   return true;
Index: 

[PATCH] D139086: [AArch64] Implement __arm_rsr128/__arm_wsr128

[Sam Elliott 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 rG83b3304dd2a3: [AArch64] Implement __arm_rsr128/__arm_wsr128 
(authored by lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139086

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/arm_acle.h
  clang/test/CodeGen/arm_acle.c
  clang/test/Preprocessor/aarch64-target-features.c
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/test/CodeGen/AArch64/aarch64-sysreg128.ll

Index: llvm/test/CodeGen/AArch64/aarch64-sysreg128.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/aarch64-sysreg128.ll
@@ -0,0 +1,48 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple aarch64-mattr=+d128 | FileCheck %s --check-prefixes=CHECK-LE
+; RUN: llc < %s -mtriple aarch64_be -mattr=+d128 | FileCheck %s --check-prefixes=CHECK-BE
+
+define i128 @test_rsr128() #0 {
+; CHECK-LE-LABEL: test_rsr128:
+; CHECK-LE:   // %bb.0: // %entry
+; CHECK-LE-NEXT:mrrs x0, x1, S1_2_C3_C4_5
+; CHECK-LE-NEXT:ret
+;
+; CHECK-BE-LABEL: test_rsr128:
+; CHECK-BE:   // %bb.0: // %entry
+; CHECK-BE-NEXT:mrrs x2, x3, S1_2_C3_C4_5
+; CHECK-BE-NEXT:mov x0, x3
+; CHECK-BE-NEXT:mov x1, x2
+; CHECK-BE-NEXT:ret
+entry:
+  %0 = call i128 @llvm.read_volatile_register.i128(metadata !1)
+  ret i128 %0
+}
+
+declare i128 @llvm.read_volatile_register.i128(metadata) #1
+
+define void @test_wsr128(i128 noundef %v) #0 {
+; CHECK-LE-LABEL: test_wsr128:
+; CHECK-LE:   // %bb.0: // %entry
+; CHECK-LE-NEXT:// kill: def $x1 killed $x1 killed $x0_x1 def $x0_x1
+; CHECK-LE-NEXT:// kill: def $x0 killed $x0 killed $x0_x1 def $x0_x1
+; CHECK-LE-NEXT:msrr S1_2_C3_C4_5, x0, x1
+; CHECK-LE-NEXT:ret
+;
+; CHECK-BE-LABEL: test_wsr128:
+; CHECK-BE:   // %bb.0: // %entry
+; CHECK-BE-NEXT:mov x2, x1
+; CHECK-BE-NEXT:mov x3, x0
+; CHECK-BE-NEXT:msrr S1_2_C3_C4_5, x2, x3
+; CHECK-BE-NEXT:ret
+entry:
+  call void @llvm.write_register.i128(metadata !1, i128 %v)
+  ret void
+}
+
+declare void @llvm.write_register.i128(metadata, i128) #1
+
+attributes #0 = { noinline nounwind }
+attributes #1 = { nounwind }
+
+!1 = !{!"1:2:3:4:5"}
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.h
===
--- llvm/lib/Target/AArch64/AArch64ISelLowering.h
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.h
@@ -430,6 +430,12 @@
   // the caller
   ASSERT_ZEXT_BOOL,
 
+  // 128-bit system register accesses
+  // lo64, hi64, chain = MRRS(chain, sysregname)
+  MRRS,
+  // chain = MSRR(chain, sysregname, lo64, hi64)
+  MSRR,
+
   // Strict (exception-raising) floating point comparison
   STRICT_FCMP = ISD::FIRST_TARGET_STRICTFP_OPCODE,
   STRICT_FCMPE,
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
===
--- llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -1627,6 +1627,11 @@
   setIndexedStoreAction(im, VT, Legal);
 }
   }
+
+  if (Subtarget->hasD128()) {
+setOperationAction(ISD::READ_REGISTER, MVT::i128, Custom);
+setOperationAction(ISD::WRITE_REGISTER, MVT::i128, Custom);
+  }
 }
 
 bool AArch64TargetLowering::shouldExpandGetActiveLaneMask(EVT ResVT,
@@ -2491,6 +2496,8 @@
 MAKE_CASE(AArch64ISD::MOPS_MEMCOPY)
 MAKE_CASE(AArch64ISD::MOPS_MEMMOVE)
 MAKE_CASE(AArch64ISD::CALL_BTI)
+MAKE_CASE(AArch64ISD::MRRS)
+MAKE_CASE(AArch64ISD::MSRR)
   }
 #undef MAKE_CASE
   return nullptr;
@@ -5943,6 +5950,26 @@
 return DAG.getNode(Op.getOpcode(), DL, {Op.getValueType(), MVT::Other},
{Ext.getValue(1), Ext.getValue(0)});
   }
+  case ISD::WRITE_REGISTER: {
+assert(Op.getOperand(2).getValueType() == MVT::i128 &&
+   "WRITE_REGISTER custom lowering is only for 128-bit sysregs");
+SDLoc DL(Op);
+
+SDValue Chain = Op.getOperand(0);
+SDValue SysRegName = Op.getOperand(1);
+SDValue Pair = Op.getOperand(2);
+
+SDValue PairLo = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i64, Pair,
+ DAG.getConstant(0, DL, MVT::i32));
+SDValue PairHi = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i64, Pair,
+ DAG.getConstant(1, DL, MVT::i32));
+
+// chain = MSRR(chain, sysregname, lo, hi)
+SDValue Result = DAG.getNode(AArch64ISD::MSRR, DL, MVT::Other, Chain,
+ SysRegName, PairLo, PairHi);
+
+return Result;
+  }
   }
 }
 
@@ -21734,6 +21761,26 @@
 }
 }
   }
+  case 

[PATCH] D139086: [AArch64] Implement __arm_rsr128/__arm_wsr128

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
lenary requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This only contains the SelectionDAG implementation. GlobalISel to
follow.

The broad approach is:

- Introduce new builtins for 128-bit wide instructions.
- Lower these to @llvm.read_register.i128/@llvm.write_register.i128
- Introduce target-specific ISD nodes which have legal operands (two i64s 
rather than an i128). These are named AArch64::{MRRS, MSRR} to match the 
instructions they are for. These are a little complex as they need to match the 
"shape" of what they're replacing or the legaliser complains.
- Select these using the existing tryReadRegister/tryWriteRegister to share the 
MDString parsing code, and introduce additional code to ensure these are 
selected into the right MRRS/MSRR instructions. What makes this hard is 
ensuring that the two i64s end up in an XSeqPair register pair, because 
SelectionDAG doesn't care that much about register classes if it can avoid 
doing so.

The main change to existing code is the reorganisation of
tryReadRegister and tryWriteRegister to try to keep the string parsing
code separate from the instruction creating code.

This also includes the changes to clang to define and use the ACLE
feature macro named `__ARM_FEATURE_SYSREG128`.

Contributors:

  Sam Elliott
  Lucas Prates


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139086

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/arm_acle.h
  clang/test/CodeGen/arm_acle.c
  clang/test/Preprocessor/aarch64-target-features.c
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/test/CodeGen/AArch64/aarch64-sysreg128.ll

Index: llvm/test/CodeGen/AArch64/aarch64-sysreg128.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/aarch64-sysreg128.ll
@@ -0,0 +1,48 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple aarch64-mattr=+d128 | FileCheck %s --check-prefixes=CHECK-LE
+; RUN: llc < %s -mtriple aarch64_be -mattr=+d128 | FileCheck %s --check-prefixes=CHECK-BE
+
+define i128 @test_rsr128() #0 {
+; CHECK-LE-LABEL: test_rsr128:
+; CHECK-LE:   // %bb.0: // %entry
+; CHECK-LE-NEXT:mrrs x0, x1, S1_2_C3_C4_5
+; CHECK-LE-NEXT:ret
+;
+; CHECK-BE-LABEL: test_rsr128:
+; CHECK-BE:   // %bb.0: // %entry
+; CHECK-BE-NEXT:mrrs x2, x3, S1_2_C3_C4_5
+; CHECK-BE-NEXT:mov x0, x3
+; CHECK-BE-NEXT:mov x1, x2
+; CHECK-BE-NEXT:ret
+entry:
+  %0 = call i128 @llvm.read_volatile_register.i128(metadata !1)
+  ret i128 %0
+}
+
+declare i128 @llvm.read_volatile_register.i128(metadata) #1
+
+define void @test_wsr128(i128 noundef %v) #0 {
+; CHECK-LE-LABEL: test_wsr128:
+; CHECK-LE:   // %bb.0: // %entry
+; CHECK-LE-NEXT:// kill: def $x1 killed $x1 killed $x0_x1 def $x0_x1
+; CHECK-LE-NEXT:// kill: def $x0 killed $x0 killed $x0_x1 def $x0_x1
+; CHECK-LE-NEXT:msrr S1_2_C3_C4_5, x0, x1
+; CHECK-LE-NEXT:ret
+;
+; CHECK-BE-LABEL: test_wsr128:
+; CHECK-BE:   // %bb.0: // %entry
+; CHECK-BE-NEXT:mov x2, x1
+; CHECK-BE-NEXT:mov x3, x0
+; CHECK-BE-NEXT:msrr S1_2_C3_C4_5, x2, x3
+; CHECK-BE-NEXT:ret
+entry:
+  call void @llvm.write_register.i128(metadata !1, i128 %v)
+  ret void
+}
+
+declare void @llvm.write_register.i128(metadata, i128) #1
+
+attributes #0 = { noinline nounwind }
+attributes #1 = { nounwind }
+
+!1 = !{!"1:2:3:4:5"}
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.h
===
--- llvm/lib/Target/AArch64/AArch64ISelLowering.h
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.h
@@ -430,6 +430,12 @@
   // the caller
   ASSERT_ZEXT_BOOL,
 
+  // 128-bit system register accesses
+  // lo64, hi64, chain = MRRS(chain, sysregname)
+  MRRS,
+  // chain = MSRR(chain, sysregname, lo64, hi64)
+  MSRR,
+
   // Strict (exception-raising) floating point comparison
   STRICT_FCMP = ISD::FIRST_TARGET_STRICTFP_OPCODE,
   STRICT_FCMPE,
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
===
--- llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -1600,6 +1600,11 @@
   setIndexedStoreAction(im, VT, Legal);
 }
   }
+
+  if (Subtarget->hasD128()) {
+setOperationAction(ISD::READ_REGISTER, MVT::i128, Custom);
+setOperationAction(ISD::WRITE_REGISTER, MVT::i128, Custom);
+  }
 }
 
 bool AArch64TargetLowering::shouldExpandGetActiveLaneMask(EVT ResVT,
@@ -2463,6 +2468,8 @@
 MAKE_CASE(AArch64ISD::MOPS_MEMCOPY)
 MAKE_CASE(AArch64ISD::MOPS_MEMMOVE)
 

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137836#3959759 , @lenary wrote:

> In D137836#3959693 , @DavidSpickett 
> wrote:
>
>> Looks like tests need updating for that new -1 return value: 
>> https://lab.llvm.org/buildbot/#/builders/178/builds/3419
>>
>> That bot builds with threading disabled.
>
> Thanks for letting me know. I'm thinking about the fix, likely an update to 
> `isThreadingSupportedArchAndOs`.

https://reviews.llvm.org/D139015 Patch here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

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

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137836#3959693 , @DavidSpickett 
wrote:

> Looks like tests need updating for that new -1 return value: 
> https://lab.llvm.org/buildbot/#/builders/178/builds/3419
>
> That bot builds with threading disabled.

Thanks for letting me know. I'm thinking about the fix, likely an update to 
`isThreadingSupportedArchAndOs`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

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

[PATCH] D138920: [AArch64] Assembly support for VMSA

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/include/llvm/Support/AArch64TargetParser.h:82
+  AEK_THE = 1ULL << 50, // FEAT_THE
+  AEK_D128 =1ULL << 51, // FEAT_LSE128
+  AEK_LSE128 =  1ULL << 52, // FEAT_D128

Comments don't correspond to names :( 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138920

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

[PATCH] D138920: [AArch64] Assembly support for VMSA

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Two nits. One below, the other: Please include everyone who contributed to this 
patch in the patch description.




Comment at: llvm/lib/Target/AArch64/AArch64.td:519
+def FeatureTHE : SubtargetFeature<"the", "HasTHE",
+"true", "Enable Translation Hardening Extension">;
+

Please can you include the feature name?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138920

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

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
lenary marked an inline comment as done.
Closed by commit rG3c97f6cab92f: [Support] Move getHostNumPhysicalCores to 
Threading.h (authored by lenary).

Changed prior to commit:
  https://reviews.llvm.org/D137836?vs=477975=478537#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

Files:
  clang-tools-extra/clangd/test/Inputs/BenchmarkHeader.h
  llvm/include/llvm/Support/Host.h
  llvm/include/llvm/Support/Threading.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/Threading.cpp
  llvm/lib/Support/Unix/Threading.inc
  llvm/lib/Support/Windows/Threading.inc
  llvm/unittests/Support/Host.cpp
  llvm/unittests/Support/Threading.cpp

Index: llvm/unittests/Support/Threading.cpp
===
--- llvm/unittests/Support/Threading.cpp
+++ llvm/unittests/Support/Threading.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include "llvm/Support/Threading.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Support/Host.h"
 #include "llvm/Support/thread.h"
 #include "gtest/gtest.h"
 
@@ -17,6 +19,18 @@
 
 namespace {
 
+static bool isThreadingSupportedArchAndOS() {
+  Triple Host(Triple::normalize(sys::getProcessTriple()));
+
+  // Initially this is only testing detection of the number of
+  // physical cores, which is currently only supported/tested on
+  // some systems.
+  return (Host.isOSWindows() && llvm_is_multithreaded()) || Host.isOSDarwin() ||
+ (Host.isX86() && Host.isOSLinux()) ||
+ (Host.isOSLinux() && !Host.isAndroid()) ||
+ (Host.isSystemZ() && Host.isOSzOS());
+}
+
 TEST(Threading, PhysicalConcurrency) {
   auto Num = heavyweight_hardware_concurrency();
   // Since Num is unsigned this will also catch us trying to
@@ -25,6 +39,20 @@
 hardware_concurrency().compute_thread_count());
 }
 
+TEST(Threading, NumPhysicalCoresSupported) {
+  if (!isThreadingSupportedArchAndOS())
+GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_GT(Num, 0);
+}
+
+TEST(Threading, NumPhysicalCoresUnsupported) {
+  if (isThreadingSupportedArchAndOS())
+GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_EQ(Num, -1);
+}
+
 #if LLVM_ENABLE_THREADS
 
 class Notification {
@@ -91,4 +119,4 @@
 #endif
 #endif
 
-} // end anon namespace
+} // namespace
Index: llvm/unittests/Support/Host.cpp
===
--- llvm/unittests/Support/Host.cpp
+++ llvm/unittests/Support/Host.cpp
@@ -30,37 +30,6 @@
 
 using namespace llvm;
 
-class HostTest : public testing::Test {
-  Triple Host;
-
-protected:
-  bool isSupportedArchAndOS() {
-// Initially this is only testing detection of the number of
-// physical cores, which is currently only supported/tested on
-// some systems.
-return (Host.isOSWindows() && llvm_is_multithreaded()) ||
-   Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
-   (Host.isOSLinux() && !Host.isAndroid()) ||
-   (Host.isSystemZ() && Host.isOSzOS());
-  }
-
-  HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
-};
-
-TEST_F(HostTest, NumPhysicalCoresSupported) {
-  if (!isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_GT(Num, 0);
-}
-
-TEST_F(HostTest, NumPhysicalCoresUnsupported) {
-  if (isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_EQ(Num, -1);
-}
-
 TEST(getLinuxHostCPUName, ARM) {
   StringRef CortexA9ProcCpuinfo = R"(
 processor   : 0
@@ -439,13 +408,13 @@
   return Success;
 }
 
-TEST_F(HostTest, DummyRunAndGetCommandOutputUse) {
+TEST(HostTest, DummyRunAndGetCommandOutputUse) {
   // Suppress defined-but-not-used warnings when the tests using the helper are
   // disabled.
   (void)
 }
 
-TEST_F(HostTest, getMacOSHostVersion) {
+TEST(HostTest, getMacOSHostVersion) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (!HostTriple.isMacOSX())
 GTEST_SKIP();
@@ -491,7 +460,7 @@
   .getOSVersion();
 }
 
-TEST_F(HostTest, AIXHostVersionDetect) {
+TEST(HostTest, AIXHostVersionDetect) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (HostTriple.getOS() != Triple::AIX)
 GTEST_SKIP();
@@ -517,7 +486,7 @@
   ASSERT_EQ(SysMinor, HostVersion.getMinor());
 }
 
-TEST_F(HostTest, AIXTargetVersionDetect) {
+TEST(HostTest, AIXTargetVersionDetect) {
   llvm::Triple TargetTriple(llvm::sys::getDefaultTargetTriple());
   if (TargetTriple.getOS() != Triple::AIX)
 GTEST_SKIP();
@@ -535,7 +504,7 @@
   ASSERT_EQ(SystemVersion.getMinor(), TargetVersion.getMinor());
 }
 
-TEST_F(HostTest, AIXHostCPUDetect) {
+TEST(HostTest, AIXHostCPUDetect) {
   llvm::Triple 

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

lenary marked 3 inline comments as done.
lenary added a comment.

Going to attempt to land this again today.




Comment at: llvm/lib/Support/Threading.cpp:59
   if (MaxThreadCount <= 0)
 MaxThreadCount = 1;
   if (ThreadsRequested == 0)

tmatheson wrote:
> It looks like this is the only place `get_physical_cores` is used, and if the 
> number is unknown or threading is disabled `MaxThreadCount` is just set to 1. 
> Would it not make sense to change `get_physical_cores` to return 1, like 
> `compute_thread_count`?
I'd rather keep the unknown semantic, so it's clearer why someone is getting 
the answer they are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

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

[PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

> As for the name TargetParser. @arsenm came up with the name 
> SubtargetRegistry. I am fine with either names.

@fpetrogalli I'm going to stick with TargetParser, because I think this is less 
confusing, given that TargetRegistry is already a component.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: clang/lib/Lex/CMakeLists.txt:3
 
-set(LLVM_LINK_COMPONENTS support)
+set(LLVM_LINK_COMPONENTS
+  Support

fpetrogalli wrote:
> I wonder how `support` was not being detected as a linking error...
Not sure, but I still think getting the case right is useful in this patch.



Comment at: flang/tools/bbc/CMakeLists.txt:2-4
+  Passes
+  TargetParser
+  )

fpetrogalli wrote:
> nit: extra tabs
I've undone the re-indent here.



Comment at: flang/unittests/Optimizer/CMakeLists.txt:14
   ${dialect_libs}
 )
 

fpetrogalli wrote:
> Why not add `LLVMTargetParser` here?
That's better, done.



Comment at: llvm/include/llvm/ADT/Triple.h:1
-//===-- llvm/ADT/Triple.h - Target triple helper class --*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLVM_ADT_TRIPLE_H
-#define LLVM_ADT_TRIPLE_H
-
-#include "llvm/ADT/Twine.h"
-#include "llvm/Support/VersionTuple.h"
-
-// Some system headers or GCC predefined macros conflict with identifiers in
-// this file.  Undefine them here.
-#undef NetBSD
-#undef mips
-#undef sparc
-
-namespace llvm {
-
-/// Triple - Helper class for working with autoconf configuration names. For
-/// historical reasons, we also call these 'triples' (they used to contain
-/// exactly three fields).
-///
-/// Configuration names are strings in the canonical form:
-///   ARCHITECTURE-VENDOR-OPERATING_SYSTEM
-/// or
-///   ARCHITECTURE-VENDOR-OPERATING_SYSTEM-ENVIRONMENT
-///
-/// This class is used for clients which want to support arbitrary
-/// configuration names, but also want to implement certain special
-/// behavior for particular configurations. This class isolates the mapping
-/// from the components of the configuration name to well known IDs.
-///
-/// At its core the Triple class is designed to be a wrapper for a triple
-/// string; the constructor does not change or normalize the triple string.
-/// Clients that need to handle the non-canonical triples that users often
-/// specify should use the normalize method.
-///
-/// See autoconf/config.guess for a glimpse into what configuration names
-/// look like in practice.
-class Triple {
-public:
-  enum ArchType {
-UnknownArch,
-
-arm,// ARM (little endian): arm, armv.*, xscale
-armeb,  // ARM (big endian): armeb
-aarch64,// AArch64 (little endian): aarch64
-aarch64_be, // AArch64 (big endian): aarch64_be
-aarch64_32, // AArch64 (little endian) ILP32: aarch64_32
-arc,// ARC: Synopsys ARC
-avr,// AVR: Atmel AVR microcontroller
-bpfel,  // eBPF or extended BPF or 64-bit BPF (little endian)
-bpfeb,  // eBPF or extended BPF or 64-bit BPF (big endian)
-csky,   // CSKY: csky
-dxil,   // DXIL 32-bit DirectX bytecode
-hexagon,// Hexagon: hexagon
-loongarch32,// LoongArch (32-bit): loongarch32
-loongarch64,// LoongArch (64-bit): loongarch64
-m68k,   // M68k: Motorola 680x0 family
-mips,   // MIPS: mips, mipsallegrex, mipsr6
-mipsel, // MIPSEL: mipsel, mipsallegrexe, mipsr6el
-mips64, // MIPS64: mips64, mips64r6, mipsn32, mipsn32r6
-mips64el,   // MIPS64EL: mips64el, mips64r6el, mipsn32el, mipsn32r6el
-msp430, // MSP430: msp430
-ppc,// PPC: powerpc
-ppcle,  // PPCLE: powerpc (little endian)
-ppc64,  // PPC64: powerpc64, ppu
-ppc64le,// PPC64LE: powerpc64le
-r600,   // R600: AMD GPUs HD2XXX - HD6XXX
-amdgcn, // AMDGCN: AMD GCN GPUs
-riscv32,// RISC-V (32-bit): riscv32
-riscv64,// RISC-V (64-bit): riscv64
-sparc,  // Sparc: sparc
-sparcv9,// Sparcv9: Sparcv9
-sparcel,// Sparc: (endianness = little). NB: 'Sparcle' is a CPU 
variant
-systemz,// SystemZ: s390x
-tce,// TCE (http://tce.cs.tut.fi/): tce
-tcele,  // TCE little endian (http://tce.cs.tut.fi/): tcele
-thumb,  // Thumb (little endian): thumb, thumbv.*
-thumbeb,// Thumb (big endian): thumbeb
-x86,// X86: i[3-9]86
-x86_64, // X86-64: amd64, x86_64
-xcore,  // XCore: xcore
-nvptx,  // NVPTX: 32-bit
-nvptx64,// NVPTX: 64-bit
-le32,   // le32: generic little-endian 32-bit CPU (PNaCl)
-le64,   // le64: generic little-endian 64-bit CPU (PNaCl)
-amdil,  // AMDIL
-amdil64,// AMDIL with 64-bit pointers
-hsail,  // AMD HSAIL
-

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

lenary added a subscriber: MaskRay.
lenary added a comment.

New version, which changes how the move works a bit. I've updated the 
description with the caveats, so this is now definitely not NFC.

I'm looking for re-review, and maybe input from @MaskRay who has made some 
minor cleanups in this code recently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

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

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

lenary edited the summary of this revision.
lenary updated this revision to Diff 477975.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

Files:
  clang-tools-extra/clangd/test/Inputs/BenchmarkHeader.h
  llvm/include/llvm/Support/Host.h
  llvm/include/llvm/Support/Threading.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/Threading.cpp
  llvm/lib/Support/Unix/Threading.inc
  llvm/lib/Support/Windows/Threading.inc
  llvm/unittests/Support/Host.cpp
  llvm/unittests/Support/Threading.cpp

Index: llvm/unittests/Support/Threading.cpp
===
--- llvm/unittests/Support/Threading.cpp
+++ llvm/unittests/Support/Threading.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include "llvm/Support/Threading.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Support/Host.h"
 #include "llvm/Support/thread.h"
 #include "gtest/gtest.h"
 
@@ -17,6 +19,18 @@
 
 namespace {
 
+static bool isThreadingSupportedArchAndOS() {
+  Triple Host(Triple::normalize(sys::getProcessTriple()));
+
+  // Initially this is only testing detection of the number of
+  // physical cores, which is currently only supported/tested on
+  // some systems.
+  return (Host.isOSWindows() && llvm_is_multithreaded()) ||
+  Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
+  (Host.isOSLinux() && !Host.isAndroid()) ||
+  (Host.isSystemZ() && Host.isOSzOS());
+}
+
 TEST(Threading, PhysicalConcurrency) {
   auto Num = heavyweight_hardware_concurrency();
   // Since Num is unsigned this will also catch us trying to
@@ -25,6 +39,20 @@
 hardware_concurrency().compute_thread_count());
 }
 
+TEST(Threading, NumPhysicalCoresSupported) {
+  if (!isThreadingSupportedArchAndOS())
+GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_GT(Num, 0);
+}
+
+TEST(Threading, NumPhysicalCoresUnsupported) {
+  if (isThreadingSupportedArchAndOS())
+GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_EQ(Num, -1);
+}
+
 #if LLVM_ENABLE_THREADS
 
 class Notification {
Index: llvm/unittests/Support/Host.cpp
===
--- llvm/unittests/Support/Host.cpp
+++ llvm/unittests/Support/Host.cpp
@@ -30,37 +30,6 @@
 
 using namespace llvm;
 
-class HostTest : public testing::Test {
-  Triple Host;
-
-protected:
-  bool isSupportedArchAndOS() {
-// Initially this is only testing detection of the number of
-// physical cores, which is currently only supported/tested on
-// some systems.
-return (Host.isOSWindows() && llvm_is_multithreaded()) ||
-   Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
-   (Host.isOSLinux() && !Host.isAndroid()) ||
-   (Host.isSystemZ() && Host.isOSzOS());
-  }
-
-  HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
-};
-
-TEST_F(HostTest, NumPhysicalCoresSupported) {
-  if (!isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_GT(Num, 0);
-}
-
-TEST_F(HostTest, NumPhysicalCoresUnsupported) {
-  if (isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_EQ(Num, -1);
-}
-
 TEST(getLinuxHostCPUName, ARM) {
   StringRef CortexA9ProcCpuinfo = R"(
 processor   : 0
@@ -439,13 +408,13 @@
   return Success;
 }
 
-TEST_F(HostTest, DummyRunAndGetCommandOutputUse) {
+TEST(HostTest, DummyRunAndGetCommandOutputUse) {
   // Suppress defined-but-not-used warnings when the tests using the helper are
   // disabled.
   (void)
 }
 
-TEST_F(HostTest, getMacOSHostVersion) {
+TEST(HostTest, getMacOSHostVersion) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (!HostTriple.isMacOSX())
 GTEST_SKIP();
@@ -491,7 +460,7 @@
   .getOSVersion();
 }
 
-TEST_F(HostTest, AIXHostVersionDetect) {
+TEST(HostTest, AIXHostVersionDetect) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (HostTriple.getOS() != Triple::AIX)
 GTEST_SKIP();
@@ -517,7 +486,7 @@
   ASSERT_EQ(SysMinor, HostVersion.getMinor());
 }
 
-TEST_F(HostTest, AIXTargetVersionDetect) {
+TEST(HostTest, AIXTargetVersionDetect) {
   llvm::Triple TargetTriple(llvm::sys::getDefaultTargetTriple());
   if (TargetTriple.getOS() != Triple::AIX)
 GTEST_SKIP();
@@ -535,7 +504,7 @@
   ASSERT_EQ(SystemVersion.getMinor(), TargetVersion.getMinor());
 }
 
-TEST_F(HostTest, AIXHostCPUDetect) {
+TEST(HostTest, AIXHostCPUDetect) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (HostTriple.getOS() != Triple::AIX)
 GTEST_SKIP();
Index: llvm/lib/Support/Windows/Threading.inc
===
--- llvm/lib/Support/Windows/Threading.inc
+++ llvm/lib/Support/Windows/Threading.inc
@@ -238,8 +238,7 @@
   return I;
 }
 
-// for sys::getHostNumPhysicalCores
-int 

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137836#3950846 , @fhahn wrote:

> (it looks like this job should have sent an email: 
> https://green.lab.llvm.org/green/job/clang-stage1-RA/32031/console)

Yeah, sorry, no email. I'll check with colleagues to see if other people do get 
emails from this CI instance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

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

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137836#3950815 , @fhahn wrote:

> Unfortunately it looks like this commit breaks building on ARM64 macOS. I 
> reverted the change for now and added more details on the error in the revert 
> commit.

Thanks for reverting. I hadn't seen any buildbot messages about this until now. 
It's annoying there's no mac pre-merge testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

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

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott 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 rG5577207d6d3e: [Support] Move getHostNumPhysicalCores to 
Threading.h (authored by lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

Files:
  clang-tools-extra/clangd/test/Inputs/BenchmarkHeader.h
  llvm/include/llvm/Support/Host.h
  llvm/include/llvm/Support/Threading.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/Threading.cpp
  llvm/unittests/Support/Host.cpp
  llvm/unittests/Support/Threading.cpp

Index: llvm/unittests/Support/Threading.cpp
===
--- llvm/unittests/Support/Threading.cpp
+++ llvm/unittests/Support/Threading.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include "llvm/Support/Threading.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Support/Host.h"
 #include "llvm/Support/thread.h"
 #include "gtest/gtest.h"
 
@@ -17,6 +19,18 @@
 
 namespace {
 
+static bool isThreadingSupportedArchAndOS() {
+  Triple Host(Triple::normalize(sys::getProcessTriple()));
+
+  // Initially this is only testing detection of the number of
+  // physical cores, which is currently only supported/tested on
+  // some systems.
+  return (Host.isOSWindows() && llvm_is_multithreaded()) ||
+  Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
+  (Host.isOSLinux() && !Host.isAndroid()) ||
+  (Host.isSystemZ() && Host.isOSzOS());
+}
+
 TEST(Threading, PhysicalConcurrency) {
   auto Num = heavyweight_hardware_concurrency();
   // Since Num is unsigned this will also catch us trying to
@@ -25,6 +39,20 @@
 hardware_concurrency().compute_thread_count());
 }
 
+TEST(Threading, NumPhysicalCoresSupported) {
+  if (!isThreadingSupportedArchAndOS())
+GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_GT(Num, 0);
+}
+
+TEST(Threading, NumPhysicalCoresUnsupported) {
+  if (isThreadingSupportedArchAndOS())
+GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_EQ(Num, -1);
+}
+
 #if LLVM_ENABLE_THREADS
 
 class Notification {
Index: llvm/unittests/Support/Host.cpp
===
--- llvm/unittests/Support/Host.cpp
+++ llvm/unittests/Support/Host.cpp
@@ -30,37 +30,6 @@
 
 using namespace llvm;
 
-class HostTest : public testing::Test {
-  Triple Host;
-
-protected:
-  bool isSupportedArchAndOS() {
-// Initially this is only testing detection of the number of
-// physical cores, which is currently only supported/tested on
-// some systems.
-return (Host.isOSWindows() && llvm_is_multithreaded()) ||
-   Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
-   (Host.isOSLinux() && !Host.isAndroid()) ||
-   (Host.isSystemZ() && Host.isOSzOS());
-  }
-
-  HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
-};
-
-TEST_F(HostTest, NumPhysicalCoresSupported) {
-  if (!isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_GT(Num, 0);
-}
-
-TEST_F(HostTest, NumPhysicalCoresUnsupported) {
-  if (isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_EQ(Num, -1);
-}
-
 TEST(getLinuxHostCPUName, ARM) {
   StringRef CortexA9ProcCpuinfo = R"(
 processor   : 0
@@ -439,13 +408,13 @@
   return Success;
 }
 
-TEST_F(HostTest, DummyRunAndGetCommandOutputUse) {
+TEST(HostTest, DummyRunAndGetCommandOutputUse) {
   // Suppress defined-but-not-used warnings when the tests using the helper are
   // disabled.
   (void)
 }
 
-TEST_F(HostTest, getMacOSHostVersion) {
+TEST(HostTest, getMacOSHostVersion) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (!HostTriple.isMacOSX())
 GTEST_SKIP();
@@ -491,7 +460,7 @@
   .getOSVersion();
 }
 
-TEST_F(HostTest, AIXHostVersionDetect) {
+TEST(HostTest, AIXHostVersionDetect) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (HostTriple.getOS() != Triple::AIX)
 GTEST_SKIP();
@@ -517,7 +486,7 @@
   ASSERT_EQ(SysMinor, HostVersion.getMinor());
 }
 
-TEST_F(HostTest, AIXTargetVersionDetect) {
+TEST(HostTest, AIXTargetVersionDetect) {
   llvm::Triple TargetTriple(llvm::sys::getDefaultTargetTriple());
   if (TargetTriple.getOS() != Triple::AIX)
 GTEST_SKIP();
@@ -535,7 +504,7 @@
   ASSERT_EQ(SystemVersion.getMinor(), TargetVersion.getMinor());
 }
 
-TEST_F(HostTest, AIXHostCPUDetect) {
+TEST(HostTest, AIXHostCPUDetect) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (HostTriple.getOS() != Triple::AIX)
 GTEST_SKIP();
Index: llvm/lib/Support/Threading.cpp
===
--- llvm/lib/Support/Threading.cpp
+++ llvm/lib/Support/Threading.cpp
@@ -13,8 +13,12 @@
 
 #include 

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/unittests/Support/Host.cpp:33
 
-class HostTest : public testing::Test {
-  Triple Host;
-
-protected:
-  bool isSupportedArchAndOS() {
-// Initially this is only testing detection of the number of
-// physical cores, which is currently only supported/tested on
-// some systems.
-return (Host.isOSWindows() && llvm_is_multithreaded()) ||
-   Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
-   (Host.isOSLinux() && !Host.isAndroid()) ||
-   (Host.isSystemZ() && Host.isOSzOS());
-  }
-
-  HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
-};
-
-TEST_F(HostTest, NumPhysicalCoresSupported) {
-  if (!isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_GT(Num, 0);
-}
-
-TEST_F(HostTest, NumPhysicalCoresUnsupported) {
-  if (isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_EQ(Num, -1);
-}
+class HostTest : public testing::Test {};
 

fpetrogalli wrote:
> lenary wrote:
> > fpetrogalli wrote:
> > > Is this needed? 
> > My original intention was to do the minimal changes required to just move 
> > the code.
> > 
> > It's not technically needed, so I will remove it.
> Yeah, I totally get the idea of doing minimal changes. Juts mention in the 
> commit message that you have some extra cleanup that is not strictly needed 
> for the major changes of the patch, so that people blaming the change can 
> understand why that was done.
So, I forgot to submit this comment *before* I submitted the changes. If you're 
happy with how the patch looks now, +2 and I'll land it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

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

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/unittests/Support/Host.cpp:33
 
-class HostTest : public testing::Test {
-  Triple Host;
-
-protected:
-  bool isSupportedArchAndOS() {
-// Initially this is only testing detection of the number of
-// physical cores, which is currently only supported/tested on
-// some systems.
-return (Host.isOSWindows() && llvm_is_multithreaded()) ||
-   Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
-   (Host.isOSLinux() && !Host.isAndroid()) ||
-   (Host.isSystemZ() && Host.isOSzOS());
-  }
-
-  HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
-};
-
-TEST_F(HostTest, NumPhysicalCoresSupported) {
-  if (!isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_GT(Num, 0);
-}
-
-TEST_F(HostTest, NumPhysicalCoresUnsupported) {
-  if (isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_EQ(Num, -1);
-}
+class HostTest : public testing::Test {};
 

fpetrogalli wrote:
> Is this needed? 
My original intention was to do the minimal changes required to just move the 
code.

It's not technically needed, so I will remove it.



Comment at: llvm/unittests/Support/Host.cpp:401
 
 TEST_F(HostTest, DummyRunAndGetCommandOutputUse) {
   // Suppress defined-but-not-used warnings when the tests using the helper are

fpetrogalli wrote:
> I think you can remove the class `HostTest` and convert the test fixture into 
> just `TEST(` instead of `TEST_F(`. It is kind of awkward to have this: `class 
> HostTest : public testing::Test {};`
Yeah, Will do.



Comment at: llvm/unittests/Support/Threading.cpp:30
 
+class ThreadingTest : public testing::Test {
+  Triple Host;

fpetrogalli wrote:
> I don't think you need to derive from `testing:Test`? The functionality of 
> `isSupportedArchAndOS could be achieved just with a function.
Yes, will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

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

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

lenary edited the summary of this revision.
lenary updated this revision to Diff 477912.
lenary marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137836

Files:
  clang-tools-extra/clangd/test/Inputs/BenchmarkHeader.h
  llvm/include/llvm/Support/Host.h
  llvm/include/llvm/Support/Threading.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/Threading.cpp
  llvm/unittests/Support/Host.cpp
  llvm/unittests/Support/Threading.cpp

Index: llvm/unittests/Support/Threading.cpp
===
--- llvm/unittests/Support/Threading.cpp
+++ llvm/unittests/Support/Threading.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include "llvm/Support/Threading.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Support/Host.h"
 #include "llvm/Support/thread.h"
 #include "gtest/gtest.h"
 
@@ -17,6 +19,18 @@
 
 namespace {
 
+static bool isThreadingSupportedArchAndOS() {
+  Triple Host(Triple::normalize(sys::getProcessTriple()));
+
+  // Initially this is only testing detection of the number of
+  // physical cores, which is currently only supported/tested on
+  // some systems.
+  return (Host.isOSWindows() && llvm_is_multithreaded()) ||
+  Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
+  (Host.isOSLinux() && !Host.isAndroid()) ||
+  (Host.isSystemZ() && Host.isOSzOS());
+}
+
 TEST(Threading, PhysicalConcurrency) {
   auto Num = heavyweight_hardware_concurrency();
   // Since Num is unsigned this will also catch us trying to
@@ -25,6 +39,20 @@
 hardware_concurrency().compute_thread_count());
 }
 
+TEST(Threading, NumPhysicalCoresSupported) {
+  if (!isThreadingSupportedArchAndOS())
+GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_GT(Num, 0);
+}
+
+TEST(Threading, NumPhysicalCoresUnsupported) {
+  if (isThreadingSupportedArchAndOS())
+GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_EQ(Num, -1);
+}
+
 #if LLVM_ENABLE_THREADS
 
 class Notification {
Index: llvm/unittests/Support/Host.cpp
===
--- llvm/unittests/Support/Host.cpp
+++ llvm/unittests/Support/Host.cpp
@@ -30,37 +30,6 @@
 
 using namespace llvm;
 
-class HostTest : public testing::Test {
-  Triple Host;
-
-protected:
-  bool isSupportedArchAndOS() {
-// Initially this is only testing detection of the number of
-// physical cores, which is currently only supported/tested on
-// some systems.
-return (Host.isOSWindows() && llvm_is_multithreaded()) ||
-   Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
-   (Host.isOSLinux() && !Host.isAndroid()) ||
-   (Host.isSystemZ() && Host.isOSzOS());
-  }
-
-  HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
-};
-
-TEST_F(HostTest, NumPhysicalCoresSupported) {
-  if (!isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_GT(Num, 0);
-}
-
-TEST_F(HostTest, NumPhysicalCoresUnsupported) {
-  if (isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_EQ(Num, -1);
-}
-
 TEST(getLinuxHostCPUName, ARM) {
   StringRef CortexA9ProcCpuinfo = R"(
 processor   : 0
@@ -439,13 +408,13 @@
   return Success;
 }
 
-TEST_F(HostTest, DummyRunAndGetCommandOutputUse) {
+TEST(HostTest, DummyRunAndGetCommandOutputUse) {
   // Suppress defined-but-not-used warnings when the tests using the helper are
   // disabled.
   (void)
 }
 
-TEST_F(HostTest, getMacOSHostVersion) {
+TEST(HostTest, getMacOSHostVersion) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (!HostTriple.isMacOSX())
 GTEST_SKIP();
@@ -491,7 +460,7 @@
   .getOSVersion();
 }
 
-TEST_F(HostTest, AIXHostVersionDetect) {
+TEST(HostTest, AIXHostVersionDetect) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (HostTriple.getOS() != Triple::AIX)
 GTEST_SKIP();
@@ -517,7 +486,7 @@
   ASSERT_EQ(SysMinor, HostVersion.getMinor());
 }
 
-TEST_F(HostTest, AIXTargetVersionDetect) {
+TEST(HostTest, AIXTargetVersionDetect) {
   llvm::Triple TargetTriple(llvm::sys::getDefaultTargetTriple());
   if (TargetTriple.getOS() != Triple::AIX)
 GTEST_SKIP();
@@ -535,7 +504,7 @@
   ASSERT_EQ(SystemVersion.getMinor(), TargetVersion.getMinor());
 }
 
-TEST_F(HostTest, AIXHostCPUDetect) {
+TEST(HostTest, AIXHostCPUDetect) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (HostTriple.getOS() != Triple::AIX)
 GTEST_SKIP();
Index: llvm/lib/Support/Threading.cpp
===
--- llvm/lib/Support/Threading.cpp
+++ llvm/lib/Support/Threading.cpp
@@ -13,8 +13,12 @@
 
 #include "llvm/Support/Threading.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include 

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParserCommon

[Sam Elliott 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 rG3e9b6adfc7ca: [ARM] Move ARM::parseBranchProtection into 
ARMTargetParserCommon (authored by lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137835

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/Support/ARMTargetParserCommon.h
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/ARMTargetParserCommon.cpp
  llvm/lib/Support/TargetParser.cpp

Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -337,51 +337,3 @@
 
 } // namespace RISCV
 } // namespace llvm
-
-// Parse a branch protection specification, which has the form
-//   standard | none | [bti,pac-ret[+b-key,+leaf]*]
-// Returns true on success, with individual elements of the specification
-// returned in `PBP`. Returns false in error, with `Err` containing
-// an erroneous part of the spec.
-bool ARM::parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
-StringRef ) {
-  PBP = {"none", "a_key", false};
-  if (Spec == "none")
-return true; // defaults are ok
-
-  if (Spec == "standard") {
-PBP.Scope = "non-leaf";
-PBP.BranchTargetEnforcement = true;
-return true;
-  }
-
-  SmallVector Opts;
-  Spec.split(Opts, "+");
-  for (int I = 0, E = Opts.size(); I != E; ++I) {
-StringRef Opt = Opts[I].trim();
-if (Opt == "bti") {
-  PBP.BranchTargetEnforcement = true;
-  continue;
-}
-if (Opt == "pac-ret") {
-  PBP.Scope = "non-leaf";
-  for (; I + 1 != E; ++I) {
-StringRef PACOpt = Opts[I + 1].trim();
-if (PACOpt == "leaf")
-  PBP.Scope = "all";
-else if (PACOpt == "b-key")
-  PBP.Key = "b_key";
-else
-  break;
-  }
-  continue;
-}
-if (Opt == "")
-  Err = "";
-else
-  Err = Opt;
-return false;
-  }
-
-  return true;
-}
Index: llvm/lib/Support/ARMTargetParserCommon.cpp
===
--- llvm/lib/Support/ARMTargetParserCommon.cpp
+++ llvm/lib/Support/ARMTargetParserCommon.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ARMTargetParserCommon.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
 
 using namespace llvm;
@@ -130,3 +131,51 @@
 
   return EndianKind::INVALID;
 }
+
+// Parse a branch protection specification, which has the form
+//   standard | none | [bti,pac-ret[+b-key,+leaf]*]
+// Returns true on success, with individual elements of the specification
+// returned in `PBP`. Returns false in error, with `Err` containing
+// an erroneous part of the spec.
+bool ARM::parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
+StringRef ) {
+  PBP = {"none", "a_key", false};
+  if (Spec == "none")
+return true; // defaults are ok
+
+  if (Spec == "standard") {
+PBP.Scope = "non-leaf";
+PBP.BranchTargetEnforcement = true;
+return true;
+  }
+
+  SmallVector Opts;
+  Spec.split(Opts, "+");
+  for (int I = 0, E = Opts.size(); I != E; ++I) {
+StringRef Opt = Opts[I].trim();
+if (Opt == "bti") {
+  PBP.BranchTargetEnforcement = true;
+  continue;
+}
+if (Opt == "pac-ret") {
+  PBP.Scope = "non-leaf";
+  for (; I + 1 != E; ++I) {
+StringRef PACOpt = Opts[I + 1].trim();
+if (PACOpt == "leaf")
+  PBP.Scope = "all";
+else if (PACOpt == "b-key")
+  PBP.Key = "b_key";
+else
+  break;
+  }
+  continue;
+}
+if (Opt == "")
+  Err = "";
+else
+  Err = Opt;
+return false;
+  }
+
+  return true;
+}
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -182,19 +182,6 @@
 bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector );
 
 } // namespace RISCV
-
-namespace ARM {
-struct ParsedBranchProtection {
-  StringRef Scope;
-  StringRef Key;
-  bool BranchTargetEnforcement;
-};
-
-bool parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
-   StringRef );
-
-} // namespace ARM
-
 } // namespace llvm
 
 #endif
Index: llvm/include/llvm/Support/ARMTargetParserCommon.h
===
--- llvm/include/llvm/Support/ARMTargetParserCommon.h
+++ llvm/include/llvm/Support/ARMTargetParserCommon.h
@@ -37,6 +37,15 @@
 // Little/Big endian
 

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParserCommon

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 477904.
lenary marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137835

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/Support/ARMTargetParserCommon.h
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/ARMTargetParserCommon.cpp
  llvm/lib/Support/TargetParser.cpp

Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -337,51 +337,3 @@
 
 } // namespace RISCV
 } // namespace llvm
-
-// Parse a branch protection specification, which has the form
-//   standard | none | [bti,pac-ret[+b-key,+leaf]*]
-// Returns true on success, with individual elements of the specification
-// returned in `PBP`. Returns false in error, with `Err` containing
-// an erroneous part of the spec.
-bool ARM::parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
-StringRef ) {
-  PBP = {"none", "a_key", false};
-  if (Spec == "none")
-return true; // defaults are ok
-
-  if (Spec == "standard") {
-PBP.Scope = "non-leaf";
-PBP.BranchTargetEnforcement = true;
-return true;
-  }
-
-  SmallVector Opts;
-  Spec.split(Opts, "+");
-  for (int I = 0, E = Opts.size(); I != E; ++I) {
-StringRef Opt = Opts[I].trim();
-if (Opt == "bti") {
-  PBP.BranchTargetEnforcement = true;
-  continue;
-}
-if (Opt == "pac-ret") {
-  PBP.Scope = "non-leaf";
-  for (; I + 1 != E; ++I) {
-StringRef PACOpt = Opts[I + 1].trim();
-if (PACOpt == "leaf")
-  PBP.Scope = "all";
-else if (PACOpt == "b-key")
-  PBP.Key = "b_key";
-else
-  break;
-  }
-  continue;
-}
-if (Opt == "")
-  Err = "";
-else
-  Err = Opt;
-return false;
-  }
-
-  return true;
-}
Index: llvm/lib/Support/ARMTargetParserCommon.cpp
===
--- llvm/lib/Support/ARMTargetParserCommon.cpp
+++ llvm/lib/Support/ARMTargetParserCommon.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ARMTargetParserCommon.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
 
 using namespace llvm;
@@ -130,3 +131,51 @@
 
   return EndianKind::INVALID;
 }
+
+// Parse a branch protection specification, which has the form
+//   standard | none | [bti,pac-ret[+b-key,+leaf]*]
+// Returns true on success, with individual elements of the specification
+// returned in `PBP`. Returns false in error, with `Err` containing
+// an erroneous part of the spec.
+bool ARM::parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
+StringRef ) {
+  PBP = {"none", "a_key", false};
+  if (Spec == "none")
+return true; // defaults are ok
+
+  if (Spec == "standard") {
+PBP.Scope = "non-leaf";
+PBP.BranchTargetEnforcement = true;
+return true;
+  }
+
+  SmallVector Opts;
+  Spec.split(Opts, "+");
+  for (int I = 0, E = Opts.size(); I != E; ++I) {
+StringRef Opt = Opts[I].trim();
+if (Opt == "bti") {
+  PBP.BranchTargetEnforcement = true;
+  continue;
+}
+if (Opt == "pac-ret") {
+  PBP.Scope = "non-leaf";
+  for (; I + 1 != E; ++I) {
+StringRef PACOpt = Opts[I + 1].trim();
+if (PACOpt == "leaf")
+  PBP.Scope = "all";
+else if (PACOpt == "b-key")
+  PBP.Key = "b_key";
+else
+  break;
+  }
+  continue;
+}
+if (Opt == "")
+  Err = "";
+else
+  Err = Opt;
+return false;
+  }
+
+  return true;
+}
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -182,19 +182,6 @@
 bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector );
 
 } // namespace RISCV
-
-namespace ARM {
-struct ParsedBranchProtection {
-  StringRef Scope;
-  StringRef Key;
-  bool BranchTargetEnforcement;
-};
-
-bool parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
-   StringRef );
-
-} // namespace ARM
-
 } // namespace llvm
 
 #endif
Index: llvm/include/llvm/Support/ARMTargetParserCommon.h
===
--- llvm/include/llvm/Support/ARMTargetParserCommon.h
+++ llvm/include/llvm/Support/ARMTargetParserCommon.h
@@ -37,6 +37,15 @@
 // Little/Big endian
 EndianKind parseArchEndian(StringRef Arch);
 
+struct ParsedBranchProtection {
+  StringRef Scope;
+  StringRef Key;
+  bool BranchTargetEnforcement;
+};
+
+bool 

[PATCH] D138579: [AArch64] Assembly support for FEAT_LRCPC3

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:8572
+  def STLR_x_64 : BaseLRCPC3IntegerLoadStore<0b11, 0b10, (outs GPR64sp:$Rn_wb) 
  , (ins GPR64:$Rt, GPR64sp:$Rn), "stlr" , "\t$Rt, [$Rn, #-8]!", "$Rn = 
$Rn_wb">; /* PUSH register */
+  def LDAPR_w_32: BaseLRCPC3IntegerLoadStore<0b10, 0b11, (outs GPR32:$Rt, 
GPR64sp:$Rn_wb), (ins GPR64sp:$Rn)   , "ldapr", "\t$Rt, [$Rn], #4"  , 
"$Rn = $Rn_wb">; /* POP register */
+  def LDAPR_x_64: BaseLRCPC3IntegerLoadStore<0b11, 0b11, (outs GPR64:$Rt, 
GPR64sp:$Rn_wb), (ins GPR64sp:$Rn)   , "ldapr", "\t$Rt, [$Rn], #8"  , 
"$Rn = $Rn_wb">; /* POP register */

Additional comment: can you change the order of `(outs` here, as most writeback 
instructions seem to make their writeback register the first of the outs. A 
good example is `LDRXpost` in the fully-expanded tablegen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138579

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

[PATCH] D138579: [AArch64] Assembly support for FEAT_LRCPC3

[Sam Elliott via Phabricator via cfe-commits]

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

Some comment nits that you can fixup on commit.




Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:3913
+// are post-indexed, and the immediate values are not inside the [] brackets 
and
+// thus not verified by GPR64sp0 parser.
+def STLRW0  : InstAlias<"stlr\t$Rt, [$Rn, #0]" , (STLRW   GPR32: $Rt, 
GPR64sp:$Rn)>;

Maybe?

Maybe also clarify that `GPR64sp0` is not appropriate because it parses and 
discards the optional zero.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:8556
+  //   size   opcopc2
+  def STILPwo : BaseLRCPC3IntegerLoadStorePair<0b10, 0b00, 0b, (outs 
GPR64sp:$Rn_wb)   , (ins GPR32:$Rt, GPR32:$Rt2, 
GPR64sp:$Rn), "stilp" , "\t$Rt, $Rt2, [$Rn, #-8]!",  "$Rn = $Rn_wb">; /* PUSH 
register pair */
+  def STILPxo : BaseLRCPC3IntegerLoadStorePair<0b11, 0b00, 0b, (outs 
GPR64sp:$Rn_wb)   , (ins GPR64:$Rt, GPR64:$Rt2, 
GPR64sp:$Rn), "stilp" , "\t$Rt, $Rt2, [$Rn, #-16]!", "$Rn = $Rn_wb">; /* PUSH 
register pair */

nit: Please remove this comment, it doesn't correspond to anything in the spec, 
and I don't think it's helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138579

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

[PATCH] D138488: [AArch64][clang] implement 2022 General Data-Processing instructions

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Oh, actually, I have a few nits. You can fix them on commit though.




Comment at: llvm/lib/Target/AArch64/AArch64.td:497
+def FeatureCSSC : SubtargetFeature<"cssc", "HasCSSC", "true",
+  "Enable Common Short Sequence Compression (CSSC) instructions">;
+

Please use the full name in parens: FEAT_CSSC (we've done this upstream for 
most other architectural extensions).





Comment at: llvm/test/MC/AArch64/armv9.4a-v94-dp.s:1
+// RUN: llvm-mc -triple aarch64-none-linux-gnu -show-encoding 
-mattr=+v9.4a < %s  | FileCheck %s
+// RUN: not llvm-mc -triple aarch64-none-linux-gnu
-mattr=+v8.8a < %s 2>&1 | FileCheck --check-prefix=NO-CSSC-ERR %s

CSSC is actually a v8.9a feature, can you update this filename?



Comment at: llvm/test/MC/Disassembler/AArch64/armv9.4a-v94-dp.txt:1
+# RUN: llvm-mc -triple aarch64-none-linux-gnu -disassemble -mattr=+v9.4a < 
%s  | FileCheck %s
+# RUN: not llvm-mc -triple aarch64-none-linux-gnu -disassemble -mattr=+v8.8a < 
%s 2>&1 | FileCheck --check-prefix=NO-V94A-ERR %s

And this filename too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138488

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

[PATCH] D138488: [AArch64][clang] implement 2022 General Data-Processing instructions

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary 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/D138488/new/

https://reviews.llvm.org/D138488

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

[PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I've updated the patch to use forwarding headers (and to rebase past some 
changes that have happened in the interim).

The patch is still huge because of the number of places using the 
TargetParsers, which need the component added to their CMakeLists.txt.

I think after the next release, I will add `#warning` to the forwarding 
headers, so their usage can be noticed and updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParserCommon

[Sam Elliott via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:47
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/ARMTargetParser.h"
 #include "llvm/Support/CodeGen.h"

tmatheson wrote:
> For consistency this could just come via ARM.h
Not any more! Those two headers don't need ARMTargetParserCommon.h so I moved 
their use into the implementation file, and now this is much better at avoiding 
transitive includes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137835

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

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParserCommon

[Sam Elliott via Phabricator via cfe-commits]

lenary retitled this revision from "[ARM] Move ARM::parseBranchProtection into 
ARMTargetParser" to "[ARM] Move ARM::parseBranchProtection into 
ARMTargetParserCommon".
lenary edited the summary of this revision.
lenary updated this revision to Diff 475860.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137835

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/Support/ARMTargetParserCommon.h
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/ARMTargetParserCommon.cpp
  llvm/lib/Support/TargetParser.cpp

Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -337,51 +337,3 @@
 
 } // namespace RISCV
 } // namespace llvm
-
-// Parse a branch protection specification, which has the form
-//   standard | none | [bti,pac-ret[+b-key,+leaf]*]
-// Returns true on success, with individual elements of the specification
-// returned in `PBP`. Returns false in error, with `Err` containing
-// an erroneous part of the spec.
-bool ARM::parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
-StringRef ) {
-  PBP = {"none", "a_key", false};
-  if (Spec == "none")
-return true; // defaults are ok
-
-  if (Spec == "standard") {
-PBP.Scope = "non-leaf";
-PBP.BranchTargetEnforcement = true;
-return true;
-  }
-
-  SmallVector Opts;
-  Spec.split(Opts, "+");
-  for (int I = 0, E = Opts.size(); I != E; ++I) {
-StringRef Opt = Opts[I].trim();
-if (Opt == "bti") {
-  PBP.BranchTargetEnforcement = true;
-  continue;
-}
-if (Opt == "pac-ret") {
-  PBP.Scope = "non-leaf";
-  for (; I + 1 != E; ++I) {
-StringRef PACOpt = Opts[I + 1].trim();
-if (PACOpt == "leaf")
-  PBP.Scope = "all";
-else if (PACOpt == "b-key")
-  PBP.Key = "b_key";
-else
-  break;
-  }
-  continue;
-}
-if (Opt == "")
-  Err = "";
-else
-  Err = Opt;
-return false;
-  }
-
-  return true;
-}
Index: llvm/lib/Support/ARMTargetParserCommon.cpp
===
--- llvm/lib/Support/ARMTargetParserCommon.cpp
+++ llvm/lib/Support/ARMTargetParserCommon.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ARMTargetParserCommon.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
 
 using namespace llvm;
@@ -103,3 +104,51 @@
   // Arch will either be a 'v' name (v7a) or a marketing name (xscale).
   return A;
 }
+
+// Parse a branch protection specification, which has the form
+//   standard | none | [bti,pac-ret[+b-key,+leaf]*]
+// Returns true on success, with individual elements of the specification
+// returned in `PBP`. Returns false in error, with `Err` containing
+// an erroneous part of the spec.
+bool ARM::parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
+StringRef ) {
+  PBP = {"none", "a_key", false};
+  if (Spec == "none")
+return true; // defaults are ok
+
+  if (Spec == "standard") {
+PBP.Scope = "non-leaf";
+PBP.BranchTargetEnforcement = true;
+return true;
+  }
+
+  SmallVector Opts;
+  Spec.split(Opts, "+");
+  for (int I = 0, E = Opts.size(); I != E; ++I) {
+StringRef Opt = Opts[I].trim();
+if (Opt == "bti") {
+  PBP.BranchTargetEnforcement = true;
+  continue;
+}
+if (Opt == "pac-ret") {
+  PBP.Scope = "non-leaf";
+  for (; I + 1 != E; ++I) {
+StringRef PACOpt = Opts[I + 1].trim();
+if (PACOpt == "leaf")
+  PBP.Scope = "all";
+else if (PACOpt == "b-key")
+  PBP.Key = "b_key";
+else
+  break;
+  }
+  continue;
+}
+if (Opt == "")
+  Err = "";
+else
+  Err = Opt;
+return false;
+  }
+
+  return true;
+}
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -182,19 +182,6 @@
 bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector );
 
 } // namespace RISCV
-
-namespace ARM {
-struct ParsedBranchProtection {
-  StringRef Scope;
-  StringRef Key;
-  bool BranchTargetEnforcement;
-};
-
-bool parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
-   StringRef );
-
-} // namespace ARM
-
 } // namespace llvm
 
 #endif
Index: llvm/include/llvm/Support/ARMTargetParserCommon.h
===
--- llvm/include/llvm/Support/ARMTargetParserCommon.h
+++ 

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParser

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I'm going to move this into the newly created `ARMTargetParserCommon` files 
which @tmatheson created in https://reviews.llvm.org/D137924 (which has landed).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137835

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

[PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137838#3921828 , @thakis wrote:

> This is a gigantic diff. I'd recommend keeping the .h files in the old place 
> for v0 and make them just forwarding headers that include the header at the 
> new location. That way, you don't have to update oodles of include lines in 
> this patch, and it makes it a bit easier to see what's going on. (You can 
> then update all the include lines in a trivial follow-up if this change goes 
> through, and then remove the forwarding headers in Support, to cut the 
> dependency you want to remove.)

Thanks for the suggestion. I do agree this patch is too big to land as-is. I 
think this patch is useful as "this shows the full effect of this change", even 
if we find ways to make how we land this patch less invasive. One other thought 
I had was to move the unittests first, but that doesn't make as big a 
difference as the fact that `llvm/ADT/Triple.h` is referenced everywhere.

I would like more comments (either here or on Discourse, link in a prior 
comment) on whether this split is reasonable before I post a v2 of this patch, 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137516: [TargetSupport] Move TargetParser API in a separate LLVM component.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137516#3912900 , @fpetrogalli 
wrote:

> In D137516#3912842 , @lenary wrote:
>
>> In D137516#3912751 , @fpetrogalli 
>> wrote:
>>
>>> @arsenm @frasercrmck @lenary - thank you for the feedback
>>>
>>> 1. The library could host anything that needs to be auto-generated with 
>>> tablegen. If we add `MVT`s, the name `TargetSupport` becomes obsolete.
>>
>> I am in favour of splitting parts of Support that could be table-gen'd out 
>> of Support, and into supplemental libraries.
>
> +1
>
>>> 2. Therefore we could use a name like `AutoGenSupport` (or similar, I am 
>>> open to suggestions).
>>
>> I actually think this is an awful name, even worse than "Support". One of 
>> the big issues with "Support" is that it's a junk-drawer that anyone throws 
>> stuff in when they need it anywhere in LLVM, when their layering questions 
>> become more difficult. This then becomes very hard to undo, because 
>> everything at this layer depends on everything else. "AutoGenSupport" to me 
>> says "we now have two bits, the junk drawer, and the junk drawer that's auto 
>> generated" and so I feel this is is worse, as you cannot differentiate which 
>> you need based on what you're doing.
>>
>> I think we need to name things after what they're for, not how they're made.
>
> Agree.
>
>>> 3. @lenary is saying that components like `Support/Host` needs to be moved 
>>> together with the AArch64/ARM bits of the target parser. Do people see an 
>>> issue if we add files that do not need auto-generated content in a library 
>>> called `AutoGenSupport`?
>>
>> The reason I'm in favour of "TargetParser" is because we already call the 
>> classes and related files "target parsers", and most of them contain 
>> target-related parsers. I don't know why this is a bad name.
>
> I'd be very happy to use the name `TargetParser`. I initially used that 
> because I knew that moving the RSCV bits of it meant  we needed to move the 
> other parsers too, together with `Support/Host`. Therefore I was trying to 
> create a name that would have covered for both the target parser any other 
> pieces that would have been moved in it. However...
>
>> Edited to add: Once I've split out Support/Host, I'm likely to give it a 
>> less generic name, something like "HostDetection", which is more clearly 
>> what it is doing.
>
> ... you seem to imply that you want to create a second component that is 
> independent of `[TargetParser|TargetSUpport]`. If that's the case, I am even 
> more convinced that `TargetParser` is the best name for the component 
> introduced in this patch.

The patch that outlines my proposal for TargetParser is here: 
https://reviews.llvm.org/D137838 (it has dependent patches). I also added a 
comment to your RFC thread explaining the dependent patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137516

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

[PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.
Herald added a subscriber: Michael137.

Apologies to everyone who has automatically been marked as a reviewer for this 
change. There is more context for it here: 
https://discourse.llvm.org/t/targetparser-auto-generation-of-riscv-cpu-definitions/66419/4?u=lenary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
Herald added subscribers: kadircet, arphaman, hiraditya, krytarowski.
Herald added a project: All.
lenary requested review of this revision.
Herald added projects: LLVM, clang-tools-extra.
Herald added subscribers: cfe-commits, llvm-commits.

This change is focussed on simplifying `Support/Host.h` to only do
target detection. In this case, this function is close in usage to
existing functions in `Support/Threading.h`, so I moved it into there.
The function is also renamed to `llvm::get_physical_cores()` to match
the style of threading's functions.

Depends on D137835 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137836

Files:
  clang-tools-extra/clangd/test/Inputs/BenchmarkHeader.h
  llvm/include/llvm/Support/Host.h
  llvm/include/llvm/Support/Threading.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/Threading.cpp
  llvm/unittests/Support/Host.cpp
  llvm/unittests/Support/Threading.cpp

Index: llvm/unittests/Support/Threading.cpp
===
--- llvm/unittests/Support/Threading.cpp
+++ llvm/unittests/Support/Threading.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include "llvm/Support/Threading.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Support/Host.h"
 #include "llvm/Support/thread.h"
 #include "gtest/gtest.h"
 
@@ -25,6 +27,37 @@
 hardware_concurrency().compute_thread_count());
 }
 
+class ThreadingTest : public testing::Test {
+  Triple Host;
+
+protected:
+  bool isSupportedArchAndOS() {
+// Initially this is only testing detection of the number of
+// physical cores, which is currently only supported/tested on
+// some systems.
+return (Host.isOSWindows() && llvm_is_multithreaded()) ||
+   Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
+   (Host.isOSLinux() && !Host.isAndroid()) ||
+   (Host.isSystemZ() && Host.isOSzOS());
+  }
+
+  ThreadingTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
+};
+
+TEST_F(ThreadingTest, NumPhysicalCoresSupported) {
+  if (!isSupportedArchAndOS())
+GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_GT(Num, 0);
+}
+
+TEST_F(ThreadingTest, NumPhysicalCoresUnsupported) {
+  if (isSupportedArchAndOS())
+GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_EQ(Num, -1);
+}
+
 #if LLVM_ENABLE_THREADS
 
 class Notification {
Index: llvm/unittests/Support/Host.cpp
===
--- llvm/unittests/Support/Host.cpp
+++ llvm/unittests/Support/Host.cpp
@@ -30,36 +30,7 @@
 
 using namespace llvm;
 
-class HostTest : public testing::Test {
-  Triple Host;
-
-protected:
-  bool isSupportedArchAndOS() {
-// Initially this is only testing detection of the number of
-// physical cores, which is currently only supported/tested on
-// some systems.
-return (Host.isOSWindows() && llvm_is_multithreaded()) ||
-   Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
-   (Host.isOSLinux() && !Host.isAndroid()) ||
-   (Host.isSystemZ() && Host.isOSzOS());
-  }
-
-  HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
-};
-
-TEST_F(HostTest, NumPhysicalCoresSupported) {
-  if (!isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_GT(Num, 0);
-}
-
-TEST_F(HostTest, NumPhysicalCoresUnsupported) {
-  if (isSupportedArchAndOS())
-GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_EQ(Num, -1);
-}
+class HostTest : public testing::Test {};
 
 TEST(getLinuxHostCPUName, ARM) {
   StringRef CortexA9ProcCpuinfo = R"(
Index: llvm/lib/Support/Threading.cpp
===
--- llvm/lib/Support/Threading.cpp
+++ llvm/lib/Support/Threading.cpp
@@ -13,8 +13,12 @@
 
 #include "llvm/Support/Threading.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Config/config.h"
-#include "llvm/Support/Host.h"
+#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
 
 #include 
 #include 
@@ -50,8 +54,8 @@
 int computeHostNumHardwareThreads();
 
 unsigned llvm::ThreadPoolStrategy::compute_thread_count() const {
-  int MaxThreadCount = UseHyperThreads ? computeHostNumHardwareThreads()
-   : sys::getHostNumPhysicalCores();
+  int MaxThreadCount =
+  UseHyperThreads ? computeHostNumHardwareThreads() : get_physical_cores();
   if (MaxThreadCount <= 0)
 MaxThreadCount = 1;
   if (ThreadsRequested == 0)
@@ -106,3 +110,123 @@
   S.ThreadsRequested = V;
   return S;
 }
+
+#if defined(__linux__) && (defined(__i386__) || defined(__x86_64__))
+// On Linux, the number of physical cores can be computed from /proc/cpuinfo,
+// using the 

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParser

[Sam Elliott via Phabricator via cfe-commits]

lenary created this revision.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
lenary requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

This should live with the Arm targets, given they have target-specific
target parsers.

Depends on D137834 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137835

Files:
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/lib/Support/TargetParser.cpp

Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -337,51 +337,3 @@
 
 } // namespace RISCV
 } // namespace llvm
-
-// Parse a branch protection specification, which has the form
-//   standard | none | [bti,pac-ret[+b-key,+leaf]*]
-// Returns true on success, with individual elements of the specification
-// returned in `PBP`. Returns false in error, with `Err` containing
-// an erroneous part of the spec.
-bool ARM::parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
-StringRef ) {
-  PBP = {"none", "a_key", false};
-  if (Spec == "none")
-return true; // defaults are ok
-
-  if (Spec == "standard") {
-PBP.Scope = "non-leaf";
-PBP.BranchTargetEnforcement = true;
-return true;
-  }
-
-  SmallVector Opts;
-  Spec.split(Opts, "+");
-  for (int I = 0, E = Opts.size(); I != E; ++I) {
-StringRef Opt = Opts[I].trim();
-if (Opt == "bti") {
-  PBP.BranchTargetEnforcement = true;
-  continue;
-}
-if (Opt == "pac-ret") {
-  PBP.Scope = "non-leaf";
-  for (; I + 1 != E; ++I) {
-StringRef PACOpt = Opts[I + 1].trim();
-if (PACOpt == "leaf")
-  PBP.Scope = "all";
-else if (PACOpt == "b-key")
-  PBP.Key = "b_key";
-else
-  break;
-  }
-  continue;
-}
-if (Opt == "")
-  Err = "";
-else
-  Err = Opt;
-return false;
-  }
-
-  return true;
-}
Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -712,3 +712,51 @@
 
   llvm_unreachable("invalid arch name");
 }
+
+// Parse a branch protection specification, which has the form
+//   standard | none | [bti,pac-ret[+b-key,+leaf]*]
+// Returns true on success, with individual elements of the specification
+// returned in `PBP`. Returns false in error, with `Err` containing
+// an erroneous part of the spec.
+bool ARM::parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
+StringRef ) {
+  PBP = {"none", "a_key", false};
+  if (Spec == "none")
+return true; // defaults are ok
+
+  if (Spec == "standard") {
+PBP.Scope = "non-leaf";
+PBP.BranchTargetEnforcement = true;
+return true;
+  }
+
+  SmallVector Opts;
+  Spec.split(Opts, "+");
+  for (int I = 0, E = Opts.size(); I != E; ++I) {
+StringRef Opt = Opts[I].trim();
+if (Opt == "bti") {
+  PBP.BranchTargetEnforcement = true;
+  continue;
+}
+if (Opt == "pac-ret") {
+  PBP.Scope = "non-leaf";
+  for (; I + 1 != E; ++I) {
+StringRef PACOpt = Opts[I + 1].trim();
+if (PACOpt == "leaf")
+  PBP.Scope = "all";
+else if (PACOpt == "b-key")
+  PBP.Key = "b_key";
+else
+  break;
+  }
+  continue;
+}
+if (Opt == "")
+  Err = "";
+else
+  Err = Opt;
+return false;
+  }
+
+  return true;
+}
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -182,19 +182,6 @@
 bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector );
 
 } // namespace RISCV
-
-namespace ARM {
-struct ParsedBranchProtection {
-  StringRef Scope;
-  StringRef Key;
-  bool BranchTargetEnforcement;
-};
-
-bool parseBranchProtection(StringRef Spec, ParsedBranchProtection ,
-   StringRef );
-
-} // namespace ARM
-
 } // namespace llvm
 
 #endif
Index: llvm/include/llvm/Support/ARMTargetParser.h
===
--- llvm/include/llvm/Support/ARMTargetParser.h
+++ llvm/include/llvm/Support/ARMTargetParser.h
@@ -302,6 +302,15 @@
 /// string then the triple's arch name is used.
 StringRef getARMCPUForArch(const llvm::Triple , StringRef MArch = {});
 
+struct ParsedBranchProtection {
+  StringRef Scope;
+  StringRef Key;
+  bool BranchTargetEnforcement;
+};
+
+bool parseBranchProtection(StringRef 

[PATCH] D137659: [Driver] Refactor err_drv_unsupported_option_argument call sites to use llvm::opt::Arg::getSpelling

[Sam Elliott via Phabricator via cfe-commits]

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

Thanks for doing this cleanup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137659

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

[PATCH] D137516: [TargetSupport] Move TargetParser API in a separate LLVM component.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D137516#3912751 , @fpetrogalli 
wrote:

> @arsenm @frasercrmck @lenary - thank you for the feedback
>
> 1. The library could host anything that needs to be auto-generated with 
> tablegen. If we add `MVT`s, the name `TargetSupport` becomes obsolete.

I am in favour of splitting parts of Support that could be table-gen'd out of 
Support, and into supplemental libraries.

> 2. Therefore we could use a name like `AutoGenSupport` (or similar, I am open 
> to suggestions).

I actually think this is an awful name, even worse than "Support". One of the 
big issues with "Support" is that it's a junk-drawer that anyone throws stuff 
in when they need it anywhere in LLVM, when their layering questions become 
more difficult. This then becomes very hard to undo, because everything at this 
layer depends on everything else. "AutoGenSupport" to me says "we now have two 
bits, the junk drawer, and the junk drawer that's auto generated" and so I feel 
this is is worse, as you cannot differentiate which you need based on what 
you're doing.

I think we need to name things after what they're for, not how they're made.

> 3. @lenary is saying that components like `Support/Host` needs to be moved 
> together with the AArch64/ARM bits of the target parser. Do people see an 
> issue if we add files that do not need auto-generated content in a library 
> called `AutoGenSupport`?

The reason I'm in favour of "TargetParser" is because we already call the 
classes and related files "target parsers", and most of them contain 
target-related parsers. I don't know why this is a bad name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137516

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

[PATCH] D137516: [TargetSupport] Move TargetParser API in a separate LLVM component.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I'm not convinced we should be leaving any of the other Target Parser 
information in Support, if we are doing this, though this creates layering 
issues which I've been trying to unpick.

If you try to take `llvm/Support/*TargetParser*`, you find a few places where 
there are dependencies from these files to other bits of Support:

- `llvm/Support/Host` has lots of logic for turning cpu information into a 
target, to handle `-mcpu=native`. My opinion is that should also move into 
target parser. There are some parts of this file that can move into other parts 
of `llvm/Support`, like the introspection about number of cores.
- `llvm/{Support/ADT}Triple` depends on the Arm/AArch64 target parsers (because 
arm triples canonically look like `armv7a-…` or similar, and we use the 
targetparser to help canonicalise the triple). My opinion is that all this code 
can move into the target parser.

There are quite a few small cleanup patches to enable this. One of the biggest 
is that `llvm/Support/CommmandLine` wants to know the current triple and CPU 
for `--version`, even in e.g. tablegen. This was the change I most struggled 
with, but I think I have a solution that does the job fine.

I'm going to try to polish and post my patch set this week which does this 
split, as a comparison to this one.

Re the name: "TargetParser" seems reasonable to me, given that's what we've 
called the files so far anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137516

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

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Oh, can you close this revision and link it to the landed commit please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123630

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

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Reverse ping. This has been accepted, what is the status of landing this?


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

https://reviews.llvm.org/D123630

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

[PATCH] D136145: [IR][RFC] Restrict read only when cache type of llvm.prefetch is instruction

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

The Arm changes (for tests) here are reasonable, and indeed both arm 
architectures do not allocate encoding space for instruction write.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136145

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

[PATCH] D128653: [PowerPC] Fix the check for scalar MASS conversion

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

You likely need `// REQUIRES: powerpc-registered-target` in the top of the 
test, as `-enable-ppc-gen-scalar-mass` is only present if the PowerPC target 
has been compiled into LLVM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128653

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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott 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 rG1666f09933ee: [ARM] Add Support for Cortex-M85 (authored by 
lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/CodeGen/arm-target-features.c
  clang/test/Driver/arm-cortex-cpus-2.c
  clang/test/Driver/arm-nofp-disabled-features.c
  clang/test/Driver/arm-nopacbti-disabled-features.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Target/ARM/ARM.td
  llvm/test/CodeGen/ARM/build-attributes.ll
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -395,13 +395,19 @@
  ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
  ARM::AEK_FP16,
  "8.1-M.Mainline"),
+ARMCPUTestParams("cortex-m85", "armv8.1-m.main",
+ "fp-armv8-fullfp16-d16",
+ ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_SIMD |
+ ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
+ ARM::AEK_FP16 | ARM::AEK_PACBTI,
+ "8.1-M.Mainline"),
 ARMCPUTestParams("iwmmxt", "iwmmxt", "none", ARM::AEK_NONE, "iwmmxt"),
 ARMCPUTestParams("xscale", "xscale", "none", ARM::AEK_NONE, "xscale"),
 ARMCPUTestParams("swift", "armv7s", "neon-vfpv4",
  ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP,
  "7-S")));
 
-static constexpr unsigned NumARMCPUArchs = 88;
+static constexpr unsigned NumARMCPUArchs = 89;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
Index: llvm/test/CodeGen/ARM/build-attributes.ll
===
--- llvm/test/CodeGen/ARM/build-attributes.ll
+++ llvm/test/CodeGen/ARM/build-attributes.ll
@@ -235,6 +235,8 @@
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+mve.fp | FileCheck %s --check-prefix=ARMv81M-MAIN-MVEFP
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+pacbti | FileCheck %s --check-prefix=ARMv81M-MAIN-PACBTI
 ; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m55 | FileCheck %s --check-prefix=CORTEX-M55
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85 | FileCheck %s --check-prefix=CORTEX-M85
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85+nopacbti | FileCheck %s --check-prefix=CHECK-NO-PACBTI
 
 ; CPU-SUPPORTED-NOT: is not a recognized processor for this target
 
@@ -1748,6 +1750,23 @@
 ; CORTEX-M55: .eabi_attribute 38, 1
 ; CORTEX-M55: .eabi_attribute 14, 0
 
+; CORTEX-M85: .cpu cortex-m85
+; CORTEX-M85: .eabi_attribute 6, 21   @ Tag_CPU_arch
+; CORTEX-M85: .eabi_attribute 7, 77   @ Tag_CPU_arch_profile
+; CORTEX-M85: .eabi_attribute 8, 0@ Tag_ARM_ISA_use
+; CORTEX-M85: .eabi_attribute 9, 3@ Tag_THUMB_ISA_use
+; CORTEX-M85: .fpufpv5-d16
+; CORTEX-M85: .eabi_attribute 36, 1   @ Tag_FP_HP_extension
+; CORTEX-M85: .eabi_attribute 48, 2   @ Tag_MVE_arch
+; CORTEX-M85: .eabi_attribute 46, 1   @ Tag_DSP_extension
+; CORTEX-M85: .eabi_attribute 34, 1   @ Tag_CPU_unaligned_access
+; CORTEX-M85: .eabi_attribute 50, 2   @ Tag_PAC_extension
+; CORTEX-M85: .eabi_attribute 52, 2   @ Tag_BTI_extension
+
+; CHECK-NO-PACBTI-NOT: .eabi_attribute 50
+; CHECK-NO-PACBTI-NOT: .eabi_attribute 52
+
+
 define i32 @f(i64 %z) {
 ret i32 0
 }
Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -1450,6 +1450,13 @@
  HasMVEFloatOps,
  FeatureFixCMSE_CVE_2021_35465]>;
 
+def : ProcessorModel<"cortex-m85", CortexM7Model,   [ARMv81mMainline,
+ FeatureDSP,
+ FeatureFPARMv8_D16,
+ FeaturePACBTI,
+ FeatureUseMISched,
+ HasMVEFloatOps]>;
+
 def : ProcNoItin<"cortex-a32",   [ARMv8a,
  FeatureHWDivThumb,
  FeatureHWDivARM,
Index: llvm/include/llvm/Support/ARMTargetParser.def
===

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

@tschuett I've corrected the patch based on your feedback. If you're happy, I'm 
going to land this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 442109.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/CodeGen/arm-target-features.c
  clang/test/Driver/arm-cortex-cpus-2.c
  clang/test/Driver/arm-nofp-disabled-features.c
  clang/test/Driver/arm-nopacbti-disabled-features.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Target/ARM/ARM.td
  llvm/test/CodeGen/ARM/build-attributes.ll
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -395,13 +395,19 @@
  ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
  ARM::AEK_FP16,
  "8.1-M.Mainline"),
+ARMCPUTestParams("cortex-m85", "armv8.1-m.main",
+ "fp-armv8-fullfp16-d16",
+ ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_SIMD |
+ ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
+ ARM::AEK_FP16 | ARM::AEK_PACBTI,
+ "8.1-M.Mainline"),
 ARMCPUTestParams("iwmmxt", "iwmmxt", "none", ARM::AEK_NONE, "iwmmxt"),
 ARMCPUTestParams("xscale", "xscale", "none", ARM::AEK_NONE, "xscale"),
 ARMCPUTestParams("swift", "armv7s", "neon-vfpv4",
  ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP,
  "7-S")));
 
-static constexpr unsigned NumARMCPUArchs = 88;
+static constexpr unsigned NumARMCPUArchs = 89;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
Index: llvm/test/CodeGen/ARM/build-attributes.ll
===
--- llvm/test/CodeGen/ARM/build-attributes.ll
+++ llvm/test/CodeGen/ARM/build-attributes.ll
@@ -235,6 +235,8 @@
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+mve.fp | FileCheck %s --check-prefix=ARMv81M-MAIN-MVEFP
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+pacbti | FileCheck %s --check-prefix=ARMv81M-MAIN-PACBTI
 ; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m55 | FileCheck %s --check-prefix=CORTEX-M55
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85 | FileCheck %s --check-prefix=CORTEX-M85
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85+nopacbti | FileCheck %s --check-prefix=CHECK-NO-PACBTI
 
 ; CPU-SUPPORTED-NOT: is not a recognized processor for this target
 
@@ -1748,6 +1750,23 @@
 ; CORTEX-M55: .eabi_attribute 38, 1
 ; CORTEX-M55: .eabi_attribute 14, 0
 
+; CORTEX-M85: .cpu cortex-m85
+; CORTEX-M85: .eabi_attribute 6, 21   @ Tag_CPU_arch
+; CORTEX-M85: .eabi_attribute 7, 77   @ Tag_CPU_arch_profile
+; CORTEX-M85: .eabi_attribute 8, 0@ Tag_ARM_ISA_use
+; CORTEX-M85: .eabi_attribute 9, 3@ Tag_THUMB_ISA_use
+; CORTEX-M85: .fpufpv5-d16
+; CORTEX-M85: .eabi_attribute 36, 1   @ Tag_FP_HP_extension
+; CORTEX-M85: .eabi_attribute 48, 2   @ Tag_MVE_arch
+; CORTEX-M85: .eabi_attribute 46, 1   @ Tag_DSP_extension
+; CORTEX-M85: .eabi_attribute 34, 1   @ Tag_CPU_unaligned_access
+; CORTEX-M85: .eabi_attribute 50, 2   @ Tag_PAC_extension
+; CORTEX-M85: .eabi_attribute 52, 2   @ Tag_BTI_extension
+
+; CHECK-NO-PACBTI-NOT: .eabi_attribute 50
+; CHECK-NO-PACBTI-NOT: .eabi_attribute 52
+
+
 define i32 @f(i64 %z) {
 ret i32 0
 }
Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -1450,6 +1450,13 @@
  HasMVEFloatOps,
  FeatureFixCMSE_CVE_2021_35465]>;
 
+def : ProcessorModel<"cortex-m85", CortexM7Model,   [ARMv81mMainline,
+ FeatureDSP,
+ FeatureFPARMv8_D16,
+ FeaturePACBTI,
+ FeatureUseMISched,
+ HasMVEFloatOps]>;
+
 def : ProcNoItin<"cortex-a32",   [ARMv8a,
  FeatureHWDivThumb,
  FeatureHWDivARM,
Index: llvm/include/llvm/Support/ARMTargetParser.def
===
--- llvm/include/llvm/Support/ARMTargetParser.def
+++ llvm/include/llvm/Support/ARMTargetParser.def
@@ -303,6 +303,9 @@
 ARM_CPU_NAME("cortex-m35p", ARMV8MMainline,