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

Reply via email to