Pull Request - LLD Port to Patmos #4
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Emoun
reviewed
Mar 16, 2022
Emoun
reviewed
Mar 16, 2022
Emoun
reviewed
Mar 16, 2022
Emoun
reviewed
Mar 16, 2022
Emoun
reviewed
Mar 16, 2022
Emoun
reviewed
Mar 16, 2022
Emoun
reviewed
Mar 16, 2022
lld/test/ELF/Patmos/patmos-ABS32.s
Outdated
| // RUN: llvm-objdump -s --section=.data %t2 | FileCheck %s | ||
|
|
||
| // CHECK: Contents of section .data: | ||
| // ?????: S = 0x100, A = 0xfffffeff |
Emoun
reviewed
Mar 16, 2022
Emoun
reviewed
Mar 16, 2022
Emoun
reviewed
Mar 16, 2022
Emoun
reviewed
Mar 16, 2022
Emoun
reviewed
Mar 16, 2022
Emoun
requested changes
Mar 16, 2022
There was a problem hiding this comment.
@haraldschn thanks a lot for this PR. Very nice work.
I have added comments here and there that I would ask you to look at.
Emoun
approved these changes
Mar 17, 2022
There was a problem hiding this comment.
@haraldschn thanks for the changes, everything looks good. I'll get the CI running
and if everything seems good there I think we can merge it.
|
@haraldschn it seems adding lld to the build has made come quite close to the default build timeout of 6 hours. To make sure that it has enough time, I suggest we increase the timeout to 7 hours. Can I get you to do that using |
|
@Emoun is it possible to increase the timeout-minutes beyond 6 hours (without using a self-hosted runner) ? Under "Usage Limits" https://docs.github.com/en/actions/learn-github-actions/usage-limits-billing-and-administration#usage-limits a maximum of 6 hours for each job is stated. I found a discussion on github for jobs.<job_id>.timeout-minutes description is misleading github/docs#7984 |
|
Yes, it seems you are right. Never mind then, I'll merge this as-is. |
s194604
pushed a commit
to s194604/patmos-llvm-project
that referenced
this pull request
Mar 29, 2022
Andrei Matei reported a llvm11 core dump for his bpf program https://bugs.llvm.org/show_bug.cgi?id=48578 The core dump happens in LiveVariables analysis phase. t-crest#4 0x00007fce54356bb0 __restore_rt t-crest#5 0x00007fce4d51785e llvm::LiveVariables::HandleVirtRegUse(unsigned int, llvm::MachineBasicBlock*, llvm::MachineInstr&) t-crest#6 0x00007fce4d519abe llvm::LiveVariables::runOnInstr(llvm::MachineInstr&, llvm::SmallVectorImpl<unsigned int>&) t-crest#7 0x00007fce4d519ec6 llvm::LiveVariables::runOnBlock(llvm::MachineBasicBlock*, unsigned int) t-crest#8 0x00007fce4d51a4bf llvm::LiveVariables::runOnMachineFunction(llvm::MachineFunction&) The bug can be reproduced with llvm12 and latest trunk as well. Futher analysis shows that there is a bug in BPF peephole TRUNC elimination optimization, which tries to remove unnecessary TRUNC operations (a <<= 32; a >>= 32). Specifically, the compiler did wrong transformation for the following patterns: %1 = LDW ... %2 = SLL_ri %1, 32 %3 = SRL_ri %2, 32 ... %3 ... %4 = SRA_ri %2, 32 ... %4 ... The current transformation did not check how many uses of %2 and did transformation like %1 = LDW ... ... %1 ... %4 = SRL_ri %2, 32 ... %4 ... and pseudo register %2 is used by not defined and caused LiveVariables analysis core dump. To fix the issue, when traversing back from SRL_ri to SLL_ri, check to ensure SLL_ri has only one use. Otherwise, don't do transformation. Differential Revision: https://reviews.llvm.org/D97792 (cherry picked from commit 51cdb78)
s194604
pushed a commit
to s194604/patmos-llvm-project
that referenced
this pull request
Apr 10, 2022
This patch re-introduces the fix in the commit llvm/llvm-project@66b0cebf7f736 by @yrnkrn > In DwarfEHPrepare, after all passes are run, RewindFunction may be a dangling > > pointer to a dead function. To make sure it's valid, doFinalization nullptrs > RewindFunction just like the constructor and so it will be found on next run. > > llvm-svn: 217737 It seems that the fix was not migrated to `DwarfEHPrepareLegacyPass`. This patch also updates `llvm/test/CodeGen/X86/dwarf-eh-prepare.ll` to include `-run-twice` to exercise the cleanup. Without this patch `llvm-lit -v llvm/test/CodeGen/X86/dwarf-eh-prepare.ll` fails with ``` -- Testing: 1 tests, 1 workers -- FAIL: LLVM :: CodeGen/X86/dwarf-eh-prepare.ll (1 of 1) ******************** TEST 'LLVM :: CodeGen/X86/dwarf-eh-prepare.ll' FAILED ******************** Script: -- : 'RUN: at line 1'; /home/arakaki/build/llvm-project/main/bin/opt -mtriple=x86_64-linux-gnu -dwarfehprepare -simplifycfg-require-and-preserve-domtree=1 -run-twice < /home/arakaki/repos/watch/llvm-project/llvm/test/CodeGen/X86/dwarf-eh-prepare.ll -S | /home/arakaki/build/llvm-project/main/bin/FileCheck /home/arakaki/repos/watch/llvm-project/llvm/test/CodeGen/X86/dwarf-eh-prepare.ll -- Exit Code: 2 Command Output (stderr): -- Referencing function in another module! call void @_Unwind_Resume(i8* %ehptr) t-crest#1 ; ModuleID = '<stdin>' void (i8*)* @_Unwind_Resume ; ModuleID = '<stdin>' in function simple_cleanup_catch LLVM ERROR: Broken function found, compilation aborted! PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace. Stack dump: 0. Program arguments: /home/arakaki/build/llvm-project/main/bin/opt -mtriple=x86_64-linux-gnu -dwarfehprepare -simplifycfg-require-and-preserve-domtree=1 -run-twice -S 1. Running pass 'Function Pass Manager' on module '<stdin>'. 2. Running pass 'Module Verifier' on function '@simple_cleanup_catch' #0 0x000056121b570a2c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/arakaki/repos/watch/llvm-project/llvm/lib/Support/Unix/Signals.inc:569:0 t-crest#1 0x000056121b56eb64 llvm::sys::RunSignalHandlers() /home/arakaki/repos/watch/llvm-project/llvm/lib/Support/Signals.cpp:97:0 t-crest#2 0x000056121b56f28e SignalHandler(int) /home/arakaki/repos/watch/llvm-project/llvm/lib/Support/Unix/Signals.inc:397:0 t-crest#3 0x00007fc7e9b22980 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12980) t-crest#4 0x00007fc7e87d3fb7 raise /build/glibc-S7xCS9/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0 t-crest#5 0x00007fc7e87d5921 abort /build/glibc-S7xCS9/glibc-2.27/stdlib/abort.c:81:0 t-crest#6 0x000056121b4e1386 llvm::raw_svector_ostream::raw_svector_ostream(llvm::SmallVectorImpl<char>&) /home/arakaki/repos/watch/llvm-project/llvm/include/llvm/Support/raw_ostream.h:674:0 t-crest#7 0x000056121b4e1386 llvm::report_fatal_error(llvm::Twine const&, bool) /home/arakaki/repos/watch/llvm-project/llvm/lib/Support/ErrorHandling.cpp:114:0 t-crest#8 0x000056121b4e1528 (/home/arakaki/build/llvm-project/main/bin/opt+0x29e3528) t-crest#9 0x000056121adfd03f llvm::raw_ostream::operator<<(llvm::StringRef) /home/arakaki/repos/watch/llvm-project/llvm/include/llvm/Support/raw_ostream.h:218:0 FileCheck error: '<stdin>' is empty. FileCheck command line: /home/arakaki/build/llvm-project/main/bin/FileCheck /home/arakaki/repos/watch/llvm-project/llvm/test/CodeGen/X86/dwarf-eh-prepare.ll -- ******************** ******************** Failed Tests (1): LLVM :: CodeGen/X86/dwarf-eh-prepare.ll Testing Time: 0.22s Failed: 1 ``` Reviewed By: loladiro Differential Revision: https://reviews.llvm.org/D110979 (cherry picked from commit e8806d7)
Emoun
pushed a commit
that referenced
this pull request
Nov 30, 2024
Somewhat confusingly a `SCEVMulExpr` is a `SCEVNAryExpr`, so can have > 2 operands. Previously, the vscale immediate matching did not check the number of operands of the `SCEVMulExpr`, so would ignore any operands after the first two. This led to incorrect codegen (and results) for ArmSME in IREE (https://github.com/iree-org/iree), which sometimes addresses things that are a `vscale * vscale` multiple away. The test added with this change shows an example reduced from IREE. The second write should be offset from the first `16 * vscale * vscale` (* 4 bytes), however, previously LSR dropped the second vscale and instead offset the write by `#4, mul vl`, which is an offset of `16 * vscale` (* 4 bytes). (cherry picked from commit 7fad04e94b7b594389111ae7eca0883ef18dc90b)
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR request includes changes to use LLD (instead of Gold) for Linking.
The main changes in this PR are:
patmos.cpp(and class Patmos) to LLD with Implementation for the Relocation TypesDriver.cpp, InputFiles.cpp, ScriptParser.cpp, Target.cpp and Target.hto include Patmosld.lldinstead ofpatmos-ld(Gold-Linker)README-Patmosfor the testing of LLD and adaptions to include LLD during a Buildld.lldandlldwithinllvm/cmake/platforms/Patmos.cmake