[clang] [StrTable] Mechanically convert Hexagon builtins to use TableGen (PR #123460)
https://github.com/chandlerc closed https://github.com/llvm/llvm-project/pull/123460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [StrTable] Mechanically convert Hexagon builtins to use TableGen (PR #123460)
https://github.com/iajbar approved this pull request. LGTM, thanks. https://github.com/llvm/llvm-project/pull/123460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [StrTable] Mechanically convert Hexagon builtins to use TableGen (PR #123460)
iajbar wrote: Thanks @chandlerc. I am testing this patch in our downstream repo. https://github.com/llvm/llvm-project/pull/123460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [StrTable] Mechanically convert Hexagon builtins to use TableGen (PR #123460)
androm3da wrote: > > > I also so comments on the python script, but as I mentioned there, the > > > script is not code I'm suggesting to check in or maintain, merely > > > documenting for completeness. It was never written to be remotely > > > readable or clean, just to produce a verifiably equivalent TableGen file. > > > > > > Sure - that's fine, I guess I am mostly skimming it for Ikhlas' sake in > > case she wants to run something similar downstream. > > LGTM. > > Should I still wait for @iajbar to review before merging? Just wasn't super > clear from this comment. Yeah that wasn't clear, sorry. "LGTM but @iajbar should approve" Ikhlas - you should note that since Chandler claims that the actual content is preserved, that likely this should function equivalently to the baseline. But this is a good opportunity to ask questions about the transformation so that you can reproduce it downstream. https://github.com/llvm/llvm-project/pull/123460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [StrTable] Mechanically convert Hexagon builtins to use TableGen (PR #123460)
chandlerc wrote: > > I also so comments on the python script, but as I mentioned there, the > > script is not code I'm suggesting to check in or maintain, merely > > documenting for completeness. It was never written to be remotely readable > > or clean, just to produce a verifiably equivalent TableGen file. > > Sure - that's fine, I guess I am mostly skimming it for Ikhlas' sake in case > she wants to run something similar downstream. > > LGTM. Should I still wait for @iajbar to review before merging? Just wasn't super clear from this comment. (Either way, thanks for the review, appreciate it!) https://github.com/llvm/llvm-project/pull/123460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [StrTable] Mechanically convert Hexagon builtins to use TableGen (PR #123460)
androm3da wrote: > I also so comments on the python script, but as I mentioned there, the script > is not code I'm suggesting to check in or maintain, merely documenting for > completeness. It was never written to be remotely readable or clean, just to > produce a verifiably equivalent TableGen file. Sure - that's fine, I guess I am mostly skimming it for Ikhlas' sake in case she wants to run something similar downstream. LGTM. https://github.com/llvm/llvm-project/pull/123460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [StrTable] Mechanically convert Hexagon builtins to use TableGen (PR #123460)
https://github.com/chandlerc edited https://github.com/llvm/llvm-project/pull/123460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [StrTable] Mechanically convert Hexagon builtins to use TableGen (PR #123460)
chandlerc wrote:
> Thanks @chandlerc this is great! I think I'd seen multiple reviews where
> someone saw the preprocessor tricks and suggested it was more suitable for
> tablegen. Too bad we hadn't done it before now, but many thanks to you for
> doing it.
>
> I was able to do some fairly minimal regression testing with this change (I
> rebuilt the linux toolchain and ran a small subset of `llvm-test-suite` in
> `qemu-hexagon`), all passed. Unfortunately there's no architecture-specific
> builtins among those tests, so it's not a whole lot of confidence gained
> there.
>
> @iajbar can you please review this change (or perhaps it would be more useful
> to review the conversion script)? I think your review would make more sense
> than mine. Note that I did try the `Q6_V_vgetqfextor_VVR` test case that you
> recently enabled - that still compiles successfully after this change.
FWIW, I don't think extensive testing is terribly necessary here.
I verified that after this change the `.inc` file generated by TableGen has
exactly equivalent `TARGET_BUILTIN` X-macros, but in a different order. Here is
the Fish shell code I used to diff things:
```fish
diff -u (rg -I '^TARGET' BuiltinsHexagon{,Dep}.def | sd ' +' ' ' | sd ',"' ',
"' | sort | psub) (rg '^TARGET'
dev/tools/clang/include/clang/Basic/BuiltinsHexagon.inc | sd -f c
'"v([0-9]+)\|[^"]+"' 'V$1' | sd '"hvxv([0-9][0-9])\|[^"]+"' 'HVXV$1' | sd
'"hvxv79"' 'HVXV79' | sort |psub)
```
This should sort the builtins, fix different whitespace, and map the expanded
feature string literals used in TableGen to the macro names used in the `.def`
files. It uses a few less common tools like `rg`, RipGrep, and `sd`,
https://github.com/chmln/sd.
> Also, Chandler - can you update the commit message to describe which sha was
> used as the input to `hexagon_builtinify.py`? This is nice for posterity IMO.
I used the exact version deleted in this PR, from the parent commit of this
PR's commit,
https://github.com/llvm/llvm-project/commit/7253c6fde498c4c9470b681df47d46e6930d6a02
-- I will add that to the PR description as well.
I also so comments on the python script, but as I mentioned there, the script
is not code I'm suggesting to check in or maintain, merely documenting for
completeness. It was never written to be remotely readable or clean, just to
produce a verifiably equivalent TableGen file.
https://github.com/llvm/llvm-project/pull/123460
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [StrTable] Mechanically convert Hexagon builtins to use TableGen (PR #123460)
llvmbot wrote:
@llvm/pr-subscribers-backend-hexagon
Author: Chandler Carruth (chandlerc)
Changes
This switches them to use the common builtin TableGen emission.
The fancy feature string preprocessor tricks are replaced with a fairly direct
translation into TableGen.
All of the actual definitions were created using a quite hack-y Python script
that was never intended to be productionized. It preserves the order, spacing,
and even comments from the original files. For posterity, the script used is
here:
https://gist.github.com/chandlerc/f53c7d735e33eecf388529bd9a6010df
The original `.def` file appears to be generated by some out-of-tree `iset.py`
script, which because it is out of tree I couldn't update. It should be very
straightforward though to update it to generate a similar structure as was used
to produce the `.td` file.
In addition to helping move towards TableGen for all of the builtins, these
builtins in particular can be *much* more efficiently handled using TableGen
when we start emitting string tables for them because it allows de-duplicating
all of the feature strings.
---
Patch is 315.42 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/123460.diff
7 Files Affected:
- (removed) clang/include/clang/Basic/BuiltinsHexagon.def (-173)
- (added) clang/include/clang/Basic/BuiltinsHexagon.td (+2143)
- (removed) clang/include/clang/Basic/BuiltinsHexagonDep.def (-1970)
- (modified) clang/include/clang/Basic/CMakeLists.txt (+4)
- (modified) clang/include/clang/Basic/TargetBuiltins.h (+5-5)
- (modified) clang/include/module.modulemap (-2)
- (modified) clang/lib/Basic/Targets/Hexagon.cpp (+1-1)
``diff
diff --git a/clang/include/clang/Basic/BuiltinsHexagon.def
b/clang/include/clang/Basic/BuiltinsHexagon.def
deleted file mode 100644
index adff9f884c0494..00
--- a/clang/include/clang/Basic/BuiltinsHexagon.def
+++ /dev/null
@@ -1,173 +0,0 @@
-//===-- BuiltinsHexagon.def - Hexagon Builtin function database --*- 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
-//
-//===--===//
-//
-// This file defines the Hexagon-specific builtin function database. Users of
-// this file must define the BUILTIN macro to make use of this information.
-//
-//===--===//
-
-// The format of this database matches clang/Basic/Builtins.def.
-
-#if defined(BUILTIN) && !defined(TARGET_BUILTIN)
-# define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE) BUILTIN(ID, TYPE, ATTRS)
-#endif
-
-#pragma push_macro("V79")
-#define V79 "v79"
-#pragma push_macro("V75")
-#define V75 "v75|" V79
-#pragma push_macro("V73")
-#define V73 "v73|" V75
-#pragma push_macro("V71")
-#define V71 "v71|" V73
-#pragma push_macro("V69")
-#define V69 "v69|" V71
-#pragma push_macro("V68")
-#define V68 "v68|" V69
-#pragma push_macro("V67")
-#define V67 "v67|" V68
-#pragma push_macro("V66")
-#define V66 "v66|" V67
-#pragma push_macro("V65")
-#define V65 "v65|" V66
-#pragma push_macro("V62")
-#define V62 "v62|" V65
-#pragma push_macro("V60")
-#define V60 "v60|" V62
-#pragma push_macro("V55")
-#define V55 "v55|" V60
-#pragma push_macro("V5")
-#define V5 "v5|" V55
-
-#pragma push_macro("HVXV79")
-#define HVXV79 "hvxv79"
-#pragma push_macro("HVXV75")
-#define HVXV75 "hvxv75|" HVXV79
-#pragma push_macro("HVXV73")
-#define HVXV73 "hvxv73|" HVXV75
-#pragma push_macro("HVXV71")
-#define HVXV71 "hvxv71|" HVXV73
-#pragma push_macro("HVXV69")
-#define HVXV69 "hvxv69|" HVXV71
-#pragma push_macro("HVXV68")
-#define HVXV68 "hvxv68|" HVXV69
-#pragma push_macro("HVXV67")
-#define HVXV67 "hvxv67|" HVXV68
-#pragma push_macro("HVXV66")
-#define HVXV66 "hvxv66|" HVXV67
-#pragma push_macro("HVXV65")
-#define HVXV65 "hvxv65|" HVXV66
-#pragma push_macro("HVXV62")
-#define HVXV62 "hvxv62|" HVXV65
-#pragma push_macro("HVXV60")
-#define HVXV60 "hvxv60|" HVXV62
-
-
-// The builtins below are not autogenerated from iset.py.
-// Make sure you do not overwrite these.
-TARGET_BUILTIN(__builtin_SI_to_SXTHI_asrh, "ii", "", V5)
-TARGET_BUILTIN(__builtin_brev_ldd, "v*LLi*CLLi*iC", "", V5)
-TARGET_BUILTIN(__builtin_brev_ldw, "v*i*Ci*iC", "", V5)
-TARGET_BUILTIN(__builtin_brev_ldh, "v*s*Cs*iC", "", V5)
-TARGET_BUILTIN(__builtin_brev_lduh, "v*Us*CUs*iC", "", V5)
-TARGET_BUILTIN(__builtin_brev_ldb, "v*Sc*CSc*iC", "", V5)
-TARGET_BUILTIN(__builtin_brev_ldub, "v*Uc*CUc*iC", "", V5)
-TARGET_BUILTIN(__builtin_circ_ldd, "LLi*LLi*LLi*iIi", "", V5)
-TARGET_BUILTIN(__builtin_circ_ldw, "i*i*i*iIi", "", V5)
-TARGET_BUILTIN(__builtin_circ_ldh, "s*s*s*iIi", "", V5)
-TARGET_BUILTIN(__builtin_circ_lduh, "Us*Us*Us*iIi", "", V5)
-TARGET_BUILTIN(__builtin_circ_ldb, "c*c*c*iIi", "", V5)
-TARGET_BUILTIN(__builti
