User Details
- User Since
- Sep 22 2017, 3:13 PM (286 w, 6 d)
Feb 9 2022
Hi, thanks for working on this. I do like the direction this patch goes, by detecting complex operations at IR level. I do have some question though.
I'm not exactly sure why to do target specific pattern matching here. We could simply add generic complex intrinsics and the different patterns could be matched at each archs ISEL, no? I do agree that it should check in the backend if it should generate a complex operation of a given vector type.
Sep 3 2021
LGTM.
May 28 2021
@jkreiner with both llvm 12 and head I get a compiler crash in this function for file F17082111. Just run opt --dfa-jump-threading. Sorry, I couldn't reduce it further, I just don't know how to get bugpoint to work with opt.
With a llvm-12 in debug build I get:
opt --dfa-jump-threading -S < fail.ll opt: /work1/dsampaio/csw/llvm-project/llvm/include/llvm/IR/Instructions.h:2767: llvm::Value *llvm::PHINode::getIncomingValueForBlock(const llvm::BasicBlock *) const: Assertion `Idx >= 0 && "Invalid basic block argument!"' failed. PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace. Stack dump: 0. Program arguments: opt --dfa-jump-threading -S 1. Running pass 'Function Pass Manager' on module '<stdin>'. 2. Running pass 'DFA Jump Threading' on function '@main' #0 0x00007f6a3415119a llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /work1/dsampaio/csw/llvm-project/llvm/lib/Support/Unix/Signals.inc:565:11 #1 0x00007f6a3415136b PrintStackTraceSignalHandler(void*) /work1/dsampaio/csw/llvm-project/llvm/lib/Support/Unix/Signals.inc:632:1 #2 0x00007f6a3414f95b llvm::sys::RunSignalHandlers() /work1/dsampaio/csw/llvm-project/llvm/lib/Support/Signals.cpp:70:5 #3 0x00007f6a34151ae1 SignalHandler(int) /work1/dsampaio/csw/llvm-project/llvm/lib/Support/Unix/Signals.inc:407:1 #4 0x00007f6a32d7a980 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12980) #5 0x00007f6a32076fb7 raise /build/glibc-S9d2JN/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0 #6 0x00007f6a32078921 abort /build/glibc-S9d2JN/glibc-2.27/stdlib/abort.c:81:0 #7 0x00007f6a3206848a __assert_fail_base /build/glibc-S9d2JN/glibc-2.27/assert/assert.c:89:0 #8 0x00007f6a32068502 (/lib/x86_64-linux-gnu/libc.so.6+0x30502) #9 0x00007f6a344eb474 llvm::PHINode::getIncomingValueForBlock(llvm::BasicBlock const*) const /work1/dsampaio/csw/llvm-project/llvm/include/llvm/IR/Instructions.h:2768:29 #10 0x00007f6a35a37265 (anonymous namespace)::AllSwitchPaths::run() /work1/dsampaio/csw/llvm-project/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp:532:24 #11 0x00007f6a35a36826 (anonymous namespace)::DFAJumpThreading::run(llvm::Function&) /work1/dsampaio/csw/llvm-project/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp:1149:23 #12 0x00007f6a35a36c1f (anonymous namespace)::DFAJumpThreadingLegacyPass::runOnFunction(llvm::Function&) /work1/dsampaio/csw/llvm-project/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp:164:5 #13 0x00007f6a3440f84c llvm::FPPassManager::runOnFunction(llvm::Function&) /work1/dsampaio/csw/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1435:23 #14 0x00007f6a34414ac5 llvm::FPPassManager::runOnModule(llvm::Module&) /work1/dsampaio/csw/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1481:16 #15 0x00007f6a34410214 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) /work1/dsampaio/csw/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1550:23 #16 0x00007f6a3440fd38 llvm::legacy::PassManagerImpl::run(llvm::Module&) /work1/dsampaio/csw/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:541:16 #17 0x00007f6a34414dd1 llvm::legacy::PassManager::run(llvm::Module&) /work1/dsampaio/csw/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1677:3 #18 0x0000000000472e17 main /work1/dsampaio/csw/llvm-project/llvm/tools/opt/opt.cpp:998:3 #19 0x00007f6a32059bf7 __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:344:0 #20 0x000000000043577a _start (/work1/dsampaio/csw/devimage/toolchain_default/toolroot/opt/kalray/accesscore/bin/opt+0x43577a) Aborted (core dumped)
May 27 2021
I've finally got time to test this.
For our downstream arch we're seeing gains up to 27%.
Very good, thanks for working on this.
May 21 2021
Apr 20 2021
Sorry but I'm not the right person for this.
Mar 2 2021
LGTM . Just a few coding style nits to fix. But please wait a couple of days to see if someone else has anything else to say.
Hey @eastig and @amehsan, thanks for working on this. Do you have any idea when this work is going to be resumed?
Jan 13 2021
Nov 2 2020
Perhaps those were some registers that passes by in being classified inside a feature. The usual behavior in llvm (at least for arm), if I'm not mistaken, is to not do so.
So, it LGTM.
Sep 30 2020
I'm not the right reviewer for this patch.
Aug 19 2020
I'm out in vacations, just have my phone here. I'm surprised I forgot to
preserve de debug information, sorry about that.
Aug 18 2020
Feel free to move on this patch, I'm in vacations.
Aug 3 2020
Jul 30 2020
Fixed true -> false for isSOImm when checking for reducing to t1 instructions
Ping ... ping...
Jul 24 2020
Ping
Jul 22 2020
Thanks. LGTM.
Jul 21 2020
Ok, this patch seems to be correct, but it would be nice to have a test.
You can use clang -mllvm -stop-before=arm-cp-islands -mllvm --simplify-mir to obtain a machine IR before the patch, and use llc -run-pass=arm-cp-islands to validate that the alignment for the function is set to 4.
Sorry, it seems I was looking the wrong instruction, it should be the label variant: vldr.16 s0, .LCPI0_0
Hi, thanks for working on this.
What exactly are you trying to fix? From what I see from https://developer.arm.com/docs/ddi0597/h/simd-and-floating-point-instructions-alphabetic-order/vldr-immediate-load-simdfp-register-immediate
VLDR.16 s0,{pc}+0x16 requires only a alignment of 2 bytes, as it has only a single zero appended in the case of half (.16):
T1 Half-precision scalar (size == 01) (Armv8.2) VLDR{<c>}{<q>}.16 <Sd>, [<Rn> {, #{+/-}<imm>}] esize = 8 << UInt(size); add = (U == '1'); imm32 = if esize == 16 then ZeroExtend(imm8:'0', 32) else ZeroExtend(imm8:'00', 32);
Jul 17 2020
Fixed remaining of inline comments
Indeed not all of them. Fixed this time.
Jul 15 2020
Ping
Jul 10 2020
Preserve name
Jul 9 2020
Re-fixed test file, now showing only differences
Clear users assumptions (and test it)
Jul 8 2020
Fixed test location and command
Jul 7 2020
Moved to BDCE.
Rebased.
Now the AAPCS explicitly avoids conflicts with the C11, by not imposing any restriction
when the natural container will overlap a zero lenght bit-field:
https://github.com/ARM-software/abi-aa/commit/2334fc7611ede31b33e314ddd0dc90579015b322
Both 32 and 64 bit versions were updated on the same way.
Rebase
Jun 12 2020
Perhaps we could move to making half a valid type for the arm back-end as follow up patches. Allowing half as argument through the IR is already a step to that direction.
IMO this patch is already quite big and it excels in fixing the bugs it proposed.
May 27 2020
Hi @rsmith,
are you still looking into this?
cheers
May 15 2020
Addressed requests
May 14 2020
LGTM, as far @rjmccall 's concern about documentation is addressed.
Would it be possible to pass a half argument and fix-it-up at CodeGenPrepare?
May 11 2020
From my point it does LGTM.
May 5 2020
Ping
I believe we can avoid creating some blocks for latter removing them, no?
May 1 2020
Hi, @ldionne
I just submitted a what I think is the test fix. It was the only place I was not using the second element of the string and could generate a underflow.
Apr 30 2020
Apr 29 2020
Hi @davide, do you have any plans to address this issue? Regards.
Apr 27 2020
Now testing only simple-register-coalescing
Fixed x86 test
Do not perform register coalescing
Apr 25 2020
Apr 20 2020
Ping. I would like to move along this small patch.
Apr 14 2020
Ping
Hi @rnk and @efriedma, indeed my initial thoughts were as well the register allocator, specially because using the pbqp allocator there are no spills for this example so it does not fail.
From what I see in the code it seems that none of the register allocators know anything about setjmp. There's an ancient llvm-dev post confirming it: https://lists.llvm.org/pipermail/llvm-dev/2011-October/043731.html
Apr 9 2020
LGTM, not forgetting to remove the exit comments.
Hi @efriedma,
thanks for looking into this.
Apr 8 2020
Apr 6 2020
Ping
Apr 2 2020
Ping
Apr 1 2020
Mar 31 2020
Mar 27 2020
Gentle Ping. I believe all issues with this fix have been fulfilled.
Mar 23 2020
- Fixed tests to use "\\b" instead of "\b".
Mar 18 2020
LGTM
Hi,
thanks for looking into this. The patch LGTM, but regarding the indentation, I don't know what would be the best practice here. We tend to like to preserve the line-git-history, but if we start ignoring the formater check, then it has no sense in they being here.
Perhaps @t.p.northover or @olista01 could share their thoughts here.
Mar 17 2020
Mar 16 2020
Ping.
Mar 13 2020
- Removed incorrect spacing changes
- Removed unecessary changes
- Do not allow a instruction that may load or store between the LdSt[SP] and the SP update.
Mar 12 2020
- Do not use pseudo-value
Hi @EricWF, thanks for the review.
Indeed there seems to have some other unrelated issue for matching the word boundary \b. Perhaps I wrongly assumed that it should also match line beginning and end.
For example, I would expect all these matches to succeed:
Mar 11 2020
- Correct mayAlias to isStack
- Fixed tests
- Avoid optimization when target is Windows with CFI, as we need to correctly update unwind information
Ping.
Mar 10 2020
LGTM, after a nit inline.
Mar 9 2020
Do not optimize post-increment SP in windows targets