Cheri LLVM-16

[veselypeta]

Hi. I've been working on rebasing the compiler to LLVM-16. Unfortunately the changes are too big to raise as a PR so @arichardson recommend raising an issue here instead. For testing I've got all the clang/lld/llvm test suites passing (perhaps there are others) and I've managed to build and run CheriBSD purecap.

Branch: https://github.com/veselypeta/cherillvm/tree/petr/llvm-17 Target Commit: b0daacf58f417634f7c7c9496589d723592a8f5a (one before llvmorg-17-init tag)

Things still to do:

• [x] getRegAllocHints : I've disabled this for CHERI. It hits an assertion when trying to use CHERI registers. I've not looked into this deeply, but it hits an assertion when it checks that it's not got a subreg.
  • [ ] Convert all CHERI tests to opaque pointers. I've had to modify a number of tests with -opaque-pointers=0 to force the old behaviour. However, these should all now be updated to opaque pointers as of llvm-16.

To build CheriBSD I had to use an updated version of libcxx. To Build

• Checkout this branch https://github.com/veselypeta/cheribsd/tree/petr/libcxx-fixes
• And I manually replaced the /contrib/subrepo-cheri-libc++ with the libcxx directory from llvm

I'm happy to help and provide more testing or descriptions of the changes etc. Thanks.

[arichardson]

I had to apply the following patch to build and run tests:

diff --git a/llvm/unittests/CodeGen/TestAsmPrinter.h b/llvm/unittests/CodeGen/TestAsmPrinter.h
index 47ef75cbada9..db79913b17ce 100644
--- a/llvm/unittests/CodeGen/TestAsmPrinter.h
+++ b/llvm/unittests/CodeGen/TestAsmPrinter.h
@@ -30,9 +30,9 @@ public:

   MOCK_METHOD2(emitSymbolAttribute,
                bool(MCSymbol *Symbol, MCSymbolAttr Attribute));
-  MOCK_METHOD3(emitCommonSymbol,
+  MOCK_METHOD4(emitCommonSymbol,
                void(MCSymbol *Symbol, uint64_t Size, Align ByteAlignment, TailPaddingAmount TailPadding));
-  MOCK_METHOD5(emitZerofill,
+  MOCK_METHOD6(emitZerofill,
                void(MCSection *Section, MCSymbol *Symbol, uint64_t Size,
                     Align ByteAlignment, TailPaddingAmount TailPadding, SMLoc Loc));

There are two tests failing as well as a bunch of llvm-exegesis ones since I skipped that in my build and it no longer omits those.

The getRegAllocHints() crash reduces down to:

; RUN: llc -o /dev/null -mtriple=riscv64-unknown-freebsd14.0 -relocation-model=pic -mattr=+m -mattr=+a -mattr=+f -mattr=+d -mattr=+c -mattr=+xcheri -mattr=-relax -mattr=+cap-mode -mattr=-save-restore -target-abi l64pc128d -vectorize-loops -vectorize-slp -mcpu=generic-rv64 -O2 -treat-scalable-fixed-error-as-warning -verify-machineinstrs %s
target datalayout = "e-m:e-pf200:128:128:128:64-p:64:64-i64:64-i128:128-n32:64-S128-A200-P200-G200"
target triple = "riscv64-unknown-freebsd14.0"

; Function Attrs: nounwind willreturn memory(none)
declare i64 @llvm.cheri.cap.address.get.i64(ptr addrspace(200)) addrspace(200) #0

define linkonce_odr ptr addrspace(200) @_ZN9libunwind10CFI_ParserINS_17LocalAddressSpaceEE9decodeFDEERS1_u11__uintcap_tPNS2_8FDE_InfoEPNS2_8CIE_InfoEb(ptr addrspace(200) %addressSpace) addrspace(200) {
entry:
  %0 = call i64 @llvm.cheri.cap.address.get.i64(ptr addrspace(200) %addressSpace)
  %add58 = add i64 1, %0
  store i64 %add58, ptr addrspace(200) null, align 8
  ret ptr addrspace(200) null
}

attributes #0 = { nounwind willreturn memory(none) }

The "Cheri Get Address Elimination" pass removes the PsedoCGetAddr and converts it to a ADDI on a subreg (which makes sense), but it seems something later one has broken.

[veselypeta]

Thanks :). I've applied your patch. How do I reproduce your test failure? So far I've been running the check-all target, but can't see any failures.

[arichardson]

Regarding the other test failures, that is probably because I am building with cheribuild.py llvm --build-and-test (--clean) and it defaults to not building tools that we don't use. I've had to update some tests that assume presence of all tools in past so this is likely just one more of these. Should really upstream this...

[veselypeta]

Thanks Alex: the issues were 2 fold

• A mismerged .cfg file, which included the expections for llvm-exegesis not being built
• this commit, which added MCA support to RISCV target. I've disabled it if the LLVM_TOOL_LLVM_MCA_BUILD=False option is specified.

[arichardson]

I've pushed some follow-up fixes to https://github.com/arichardson/llvm-project/tree/llvm-16-test where I've marked the commit the fix should be folded into. @jrtc27 not sure if that is something you would want to fix with your rebase-with-merges script?

[veselypeta]

Any update on this? Happy to help resolve any issues.

[arichardson]

I finally got around to doing some testing with the latest version from https://github.com/arichardson/llvm-project/tree/llvm-16-test and it seems to work just fine with CheriBSD releng/23.11 with a few minor makefile changes to disable new warnings.

@jrtc27 would you be okay with pushing this to upstream-llvm-merge after folding the fixup commits into the correct initial commit? I haven't used your script yet so not sure how long it will take me to get it right but I could give it a try unless you are able to do so.

[jrtc27]

I can take a look next week when I’m back from annual leave

[veselypeta]

Ideally we would like to proceed with upgrading the version further to llvm-17. Would it cause problems to begin using alex's branch as a base or should we wait until it lands in the repo?

[arichardson]

Ideally we would like to proceed with upgrading the version further to llvm-17. Would it cause problems to begin using alex's branch as a base or should we wait until it lands in the repo?

I think we should be almost ready for this - just finishing up cleaning the history.

[arichardson]

I have started the final rebase since I made a mistake in the last one and should be able to push it once it has completed in around 2 hours.

[arichardson]

I have pushed a version with the history cleaned up to https://github.com/CTSRD-CHERI/llvm-project/tree/upstream-llvm-merge.

[arichardson]

@veselypeta FYI I just pushed a new history (same contents) to https://github.com/CTSRD-CHERI/llvm-project/tree/upstream-llvm-merge. I think once CI and CheriBSD is happy with this we can merge it to dev.

[jrtc27]

Perhaps worth posting a summary of what needed cleaning up to hopefully avoid the need in future?

[veselypeta]

Thank you for your help @arichardson. Yeah any advice/feedback would be much appreciated.

[arichardson]

Overall this is what changed: I folded some compilation fixes into the commit where the failure would have started to make bisecting more likely to work correctly. I also dropped the changes to disable the riscv regalloc changes by moving the fix after the change that introduced the assertion. Finally, a few smaller cleanups of commit messages. Overall this was about 10-15 commits that I moved around.

So in summary nothing major, just trying to keep the history in a "mostly" bisectable state.

[arichardson]

This is not somethign that needs to be done while performing merges - the fixes whenever you stop merging and test compilation is the right approach.

Having lots of small commits to fix each individual compilation issue made it a lot easier to fold the fixup into/after the upstream commit that introduced the problem was great, so I did not have to split up your commits. Thanks for all your hard work performing this merge!

[jrtc27]

I've been combing through the merge commits, discussing with Alex at points, and am finding various issues, some quite important (in particular, libunwind looks utterly broken for RISC-V and CHERI-RISC-V due to multiple cumulative mismerges such that it shouldn't even build, but definitely won't work properly on CHERI-RISC-V).

Without listing all the specifics that I've found so far, the general issues I've found multiple instances of are:

• Adding trailing whitespace
• Removing upstream's trailing whitespace (yes, it's a "fix", but those are annoying diffs to carry that serve no use)
• Mentioning your commits by hash; this means any rebasing that gets done prior to reaching dev breaks the link. Use the upstream hash instead, unless you really need to reference the merge itself, which should be rare, that's normally for referring to a mismerge done years ago.

Once I've reached the end I'll do another big rebase to fold the fixes into the relevant commits (and reword commit messages that reference rebased merges), but reviewing it all is taking a while and rebasing that large a history can be slow and fiddly.

[veselypeta]

Thanks for looking into this. I only noticed the issues with libunwind until after I had completed the merge. I have a patch to fix the build of libunwind, but it probably won't help you trying to clean up the history of the mismerges. Thanks for your advice about doing the merges, I'll keep that in mind going forward. I appreciate that I didn't keep the history very clean, especially at the start or the merge.

[arichardson]

Thanks for looking into this. I only noticed the issues with libunwind until after I had completed the merge. I have a patch to fix the build of libunwind, but it probably won't help you trying to clean up the history of the mismerges. Thanks for your advice about doing the merges, I'll keep that in mind going forward. I appreciate that I didn't keep the history very clean, especially at the start or the merge.

I have pushed the fixes for libunwind on top of the latest upstream-llvm-merge branch. We will clean up the history one more time, but that should not be a blocker for your continued merge - it can be rebased on top of the latest upstream-llvm-merge branch once completed.