Skip to content
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.

Sign up for GitHub

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

Jump to bottom

MIPS patches llvm17 #10

Merged
merged 13 commits into from
Oct 18, 2023
Merged

MIPS patches llvm17 #10

merged 13 commits into from
Oct 18, 2023

Conversation

BgZun
Copy link

@BgZun BgZun commented Sep 26, 2023

No description provided.

nrakovic and others added 5 commits September 15, 2023 10:15
@nrakovic @BgZun
[RISCV] Add i8500 scheduler and target cpu
6816fbd
@nrakovic @BgZun
Added preliminary p8700 scheduler, and an in progress version of i850…
3cd04ae 
…0 scheduler
@nrakovic @BgZun
Improved i8500 and p8700 schedulers
cf4e130
@nrakovic @BgZun
Fixed MicroOpBufferSize and LoadLatency values for p8700 scheduler.
7a126e4 
Below are coremark-pro benchmark results comparing p8700 scheduler
to the no scheduler option in llvm.

During testing certain unexpected result degradations were
encountered. These are atributed to function start alignment.
Therefore to show p8700 scheduler imporvements, another set
of results is given where the benchmakrs were run with
forced function alignment to 64 (both p8700 and no scheduler were
given this alignment).

O2
	   | no_sched ipc  | p8700 ipc     | % improvement
           -------------------------------------------------
linear     | 64908.257812  | 65255.742188  | 0.535346946
loops      | 157907.4844   | 146127.5938   | -7.459995118
nnet       | 793462.8125   | 757033.875    | -4.591133564
parser     | 372.480194    | 268.761292    | -27.84548109
radix      | 5383.770508   | 5745.964844   | 6.727521826
sha        | 10543.68945   | 12665.71094   | 20.12598621
zip        | 135.495514    | 177.480515    | 30.98626645

O3
           | no_sched ipc  | p8700 ipc     | % improvement
           -------------------------------------------------
linear     | 63470.28906   | 66298.74219   | 4.456341964
loops      | 130437.5625   | 144980.4844   | 11.14933582
nnet       | 587928.3125   | 659753.375    | 12.21663611
parser     | 371.820007    | 545.683655    | 46.76016479
radix      | 5421.005371   | 5348.705566   | -1.33369735
sha        | 12711.93164   | 8357.577148   | -34.25407417
zip        | 212.687927    | 177.436035    | -16.57446781

-----------------------------------------------------------

O2 aligned 64
           | no_sched ipc  | p8700 ipc     | % improvement
           -------------------------------------------------
linear     | 66470.16406   | 67125.09375   | 0.985298739
loops      | 173888.8906   | 189781.1875   | 9.139339965
nnet       | 833681.0625   | 842305.875    | 1.03454581
parser     | 547.239502    | 547.224182    | -0.002799506
radix      | 5404.488281   | 5714.368652   | 5.733759699
sha        | 12700.98926   | 15542.375     | 22.37137348
zip        | *	           | *             | *

O3 aligned 64
           | no_sched ipc  | p8700 ipc     | % improvement
           -------------------------------------------------
linear     | 66184.28906   | 68975.86719   | 4.217886398
loops      | 160947.5781   | 194033.1406   | 20.55673213
nnet       | 644152.0625   | 664852.5625   | 3.213604552
parser     | 459.192383    | 545.102783    | 18.70902114
radix      | 5437.531738   | 6012.506836   | 10.57419296
sha        | 12699.08203   | 15546.25098   | 22.42027368
zip        | *             | *             | *
@draganmladjenovic @BgZun
Added 'ccmov' instruction
dd65934
@BgZun BgZun marked this pull request as draft September 26, 2023 10:14
@BgZun BgZun force-pushed the local-mips-patches-llvm17 branch from 1340adb to ea86072 Compare October 3, 2023 15:13
@djtodoro djtodoro marked this pull request as ready for review October 12, 2023 13:49
jdmitrovic-syrmia
jdmitrovic-syrmia approved these changes Oct 12, 2023
View reviewed changes
Copy link

@jdmitrovic-syrmia jdmitrovic-syrmia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the code, looks good to me. One leftover comment found, but that's about it.

llvm/lib/Target/RISCV/RISCVSubtarget.h Outdated Show resolved Hide resolved
urosstajic99
urosstajic99 approved these changes Oct 12, 2023
View reviewed changes
Copy link

@urosstajic99 urosstajic99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me. I also tried to build the project and all the tests passed.

draganmladjenovic and others added 8 commits October 18, 2023 11:16
@draganmladjenovic @BgZun
[RISCV] Add mload-store-pairs support - part 1
3062893 
Supports bonding on i8500 w/o using paired instructions
@draganmladjenovic @BgZun
[clang] Add driver support for riscv64-mti toolchains
c73246a 
Default to not using integrated as and adjust assembler output
to be compatible with v0.97 gas. Rudimentary big-endian support.
@draganmladjenovic @BgZun
[RISCV] Add mload-store-pairs support - part 2
7fbc972 
Supports bonding on i8500 with using paired instructions
@BgZun
Experiment: Enable SeparateConstOffsetFromGEPPass for RISC-V
1de1c35
@draganmladjenovic @BgZun
[RISCV] Reorganize Mips cpu feature support
e0dbe1d 
Remove zba and zbb.
Allow disabling ccmov with -mno-ccmov.
Make -mload-store-pairs imply l[w|d]p and s[w|d]p.
@BgZun
[RISCV] Add RemoveBackToBackBranches Pass
4fe2285 
According to the MIPS specification, there shouldn't be
two conditional branches in the same 8-byte aligned
region of code.
@jdmitrovic-syrmia @BgZun
RISCV: EXT/INS instructions added
cd4e678
@jdmitrovic-syrmia @BgZun
Options for Load/Store pairs, EXT/INS optimizations disabled by default
fc85f61
@BgZun BgZun force-pushed the local-mips-patches-llvm17 branch from ea86072 to fc85f61 Compare October 18, 2023 10:49
@djtodoro djtodoro merged commit 3af74ae into MIPS:mti/llvm-17 Oct 18, 2023
djtodoro pushed a commit that referenced this pull request Nov 22, 2024
@Tazdevil971
[MIPS] Updated MIPS N calling conventions so that fp16 arguments no l…
8cd348c 
…onger cause a crash (#116569)

This PR fixes a bug introduced by #110199, which causes any half float
argument to crash the compiler on MIPS64.

Currently compiling this bit of code with `llc -mtriple=mips64`: 
```
define void @half_args(half %a) nounwind {
entry:
        ret void
}
```

Crashes with the following log:
```
LLVM ERROR: unable to allocate function argument #0
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: llc -mtriple=mips64
1.	Running pass 'Function Pass Manager' on module '<stdin>'.
2.	Running pass 'MIPS DAG->DAG Pattern Instruction Selection' on function '@half_args'
 #0 0x000055a3a4013df8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x32d0df8)
 #1 0x000055a3a401199e llvm::sys::RunSignalHandlers() (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x32ce99e)
 #2 0x000055a3a40144a8 SignalHandler(int) Signals.cpp:0:0
 #3 0x00007f00bde558c0 __restore_rt libc_sigaction.c:0:0
 #4 0x00007f00bdea462c __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
 #5 0x00007f00bde55822 gsignal ./signal/../sysdeps/posix/raise.c:27:6
 #6 0x00007f00bde3e4af abort ./stdlib/abort.c:81:7
 #7 0x000055a3a3f80e3c llvm::report_fatal_error(llvm::Twine const&, bool) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x323de3c)
 #8 0x000055a3a2e20dfa (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x20dddfa)
 #9 0x000055a3a2a34e20 llvm::MipsTargetLowering::LowerFormalArguments(llvm::SDValue, unsigned int, bool, llvm::SmallVectorImpl<llvm::ISD::InputArg> const&, llvm::SDLoc const&, llvm::SelectionDAG&, llvm::SmallVectorImpl<llvm::SDValue>&) const MipsISelLowering.cpp:0:0
#10 0x000055a3a3d896a9 llvm::SelectionDAGISel::LowerArguments(llvm::Function const&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30466a9)
#11 0x000055a3a3e0b3ec llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c83ec)
#12 0x000055a3a3e09e21 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c6e21)
#13 0x000055a3a2aae1ca llvm::MipsDAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) MipsISelDAGToDAG.cpp:0:0
#14 0x000055a3a3e07706 llvm::SelectionDAGISelLegacy::runOnMachineFunction(llvm::MachineFunction&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c4706)
#15 0x000055a3a3051ed6 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x230eed6)
#16 0x000055a3a35a3ec9 llvm::FPPassManager::runOnFunction(llvm::Function&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x2860ec9)
#17 0x000055a3a35ac3b2 llvm::FPPassManager::runOnModule(llvm::Module&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x28693b2)
#18 0x000055a3a35a499c llvm::legacy::PassManagerImpl::run(llvm::Module&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x286199c)
#19 0x000055a3a262abbb main (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x18e7bbb)
#20 0x00007f00bde3fc4c __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
#21 0x00007f00bde3fd05 call_init ./csu/../csu/libc-start.c:128:20
#22 0x00007f00bde3fd05 __libc_start_main@GLIBC_2.2.5 ./csu/../csu/libc-start.c:347:5
#23 0x000055a3a2624921 _start /builddir/glibc-2.39/csu/../sysdeps/x86_64/start.S:117:0
```

This is caused by the fact that after the change, `f16`s are no longer
lowered as `f32`s in calls.

Two possible fixes are available:
- Update calling conventions to properly support passing `f16` as
integers.
- Update `useFPRegsForHalfType()` to return `true` so that `f16` are
still kept in `f32` registers, as before #110199.

This PR implements the first solution to not introduce any more ABI
changes as #110199 already did.

As of what is the correct ABI for halfs, I don't think there is a
correct answer. GCC doesn't support halfs on MIPS, and I couldn't find
any information on old MIPS ABI manuals either.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdmitrovic-syrmia jdmitrovic-syrmia jdmitrovic-syrmia approved these changes

@urosstajic99 urosstajic99 urosstajic99 approved these changes

@djtodoro djtodoro djtodoro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@BgZun @djtodoro @urosstajic99 @jdmitrovic-syrmia @nrakovic @draganmladjenovic