-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support LLVM 13 #202
Support LLVM 13 #202
Conversation
I have good news and I have bad news. The good news is that I have merged #210, which will allow us to write test cases that require LLVM 13 or later. The bad news is that while writing a test case for #174, I discovered a slew of metadata-related bugs in
None of these issues are specifically about LLVM 13—they're quite old bugs that have laid dormant due to |
It took me longer than I'd hoped, but I've finally managed to add a regression test for #174—see In order to make all of the metadata in @kquick, I'm experiencing an issue with the
Versus LLVM 13:
In the latter, we run |
This brings in the changes from GaloisInc/llvm-pretty#103 and GaloisInc/llvm-pretty#108. The former requires adding an `Arbitrary` instance for `DIArgList` in `unit-test`.
This adapts to changes on the `llvm-pretty` side in GaloisInc/llvm-pretty#108.
This upholds a LLVM invariant that they are always printed inline, never in the global list of metadata. Fixes #212.
This isn't a tasty-sugar issue per-se, but the For background, if you have tasty-sugar parameters, those parameters can be present in a filename (e.g. Thus, we didn't duplicated every file: there was only the single match for So the main issue is in disasm-test/Main.hs where it handles the
|
Thanks, that explanation makes sense. I had to tweak your original suggestion to make it worked when the assumed parameter is diff --git a/disasm-test/Main.hs b/disasm-test/Main.hs
index faa1dce..0404046 100644
--- a/disasm-test/Main.hs
+++ b/disasm-test/Main.hs
@@ -307,7 +307,11 @@ runTest llvmVer sweet expct
]
in case lookup "llvm-range" (TS.expParamsMatch expct) of
Just (TS.Explicit v) -> specMatchesInstalled v
- Just (TS.Assumed v) -> specMatchesInstalled v
+ Just (TS.Assumed v)
+ | v == "pre-llvm11" || v == "at-least-llvm12"
+ -> specMatchesInstalled v
+ | otherwise
+ -> False
_ -> error "llvm-range unknown"
-- | Assemble some llvm assembly, producing a bitcode file in /tmp. But after doing that, everything works as I would expect on LLVM 11, 12, and 13. Nice! And if I understand correctly, this approach is future-proof in that I shouldn't need to modify the I've pushed the fix above, which means that this PR is finally ready for review! |
It should be! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's 13 done... just 14, 15, and 16 to go.
This is necessary to support LLVM 13. Fixes #174.
One code (`CST_CODE_INLINEASM_OLD3`) was introduced in LLVM 13, and another (`CST_CODE_INLINEASM`) was introduced in LLVM 14. For the most part, they are parsed identically to previous inline `asm` codes, but with some minor differences. I have consolidated the logic for parsing all inline `asm` codes into a single `parseInlineAsm` function. The existing `disasm-test/tests/callbr.ll` test case ensures that we handle all of these different `inline asm` codes correctly. As an added bonus, it's portable across multiple LLVM versions! Fixes #184.
This ensures that we do not run test cases with assumed `tasty-sugar` parameters multiple times with later LLVM versions.
This contains two key fixes needed to support LLVM 13:
Support
METADATA_ARG_LIST
Fixes #174.
Support newer inline
asm
codesOne code (
CST_CODE_INLINEASM_OLD3
) was introduced in LLVM 13, and another (CST_CODE_INLINEASM
) was introduced in LLVM 14. For the most part, they are parsed identically to previous inlineasm
codes, but with some minor differences. I have consolidated the logic for parsing all inlineasm
codes into a singleparseInlineAsm
function.Fixes #184.
The rest of the commits are needed to make the test case for #174 work, which exercises quite a bit of pretty-printing functionality that was not previously tested. These include fixes for GaloisInc/llvm-pretty#105, GaloisInc/llvm-pretty#107, #211, and #212. This also requires bumping the
llvm-pretty
submodule to bring in the changes from GaloisInc/llvm-pretty#108.