<div dir="ltr">Is VT a legal type on Hexagon? It looks like Hexagon may be setting SHL as Custom for every defined vector type. Try adding TLI.isTypeLegal(VT) too.</div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature" data-smartmail="gmail_signature">~Craig</div></div>
<br><div class="gmail_quote">On Thu, Sep 21, 2017 at 10:06 PM, Haidl, Michael <span dir="ltr"><<a href="mailto:michael.haidl@uni-muenster.de" target="_blank">michael.haidl@uni-muenster.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Sanjay,<br>
<br>
thanks for this information. I did get a little bit further with the<br>
patch. However, Hexagon gives me headaches.<br>
<br>
I tried to limit the scope of the patch to the BeforeLegalizeTypes phase<br>
and Hexagon still reaches the unreachable. Hexagon tries to split or<br>
widen a vector type for a node with custom lowering where the<br>
unreachable arises from inside TargetLowering::<wbr>ReplaceNodeResults which<br>
is not overwritten in the Hexagon backend.<br>
<br>
To avoid generating illegal code after all I queried TLI if all<br>
generated operations are legal operations for the given vector type.<br>
<br>
if ( TLI.isOperationLegal(ISD::SUB, VT)<br>
&& TLI.isOperationLegal(ISD::ADD, VT)<br>
&& TLI.isOperationLegal(ISD::SHL, VT)<br>
&& TLI.isOperationLegal(ISD::SRA, VT)) {<br>
<br>
The Hexagon backend says happily: yes! Go ahead!<br>
When it comes to HexagonDAGToDAGISel and the type should be legalized no<br>
custom lowering is found and TargetLowering::<wbr>ReplaceNodeResults is<br>
called which falls back to the default implementation and the<br>
unreachable is trigged.<br>
<br>
Is this really working as intended or am I missing something here?<br>
<br>
Cheers,<br>
Michael<br>
<br>
On 20.09.2017 16:10, Sanjay Patel wrote:<br>
> There are multiple problems/questions here:<br>
><br>
> 1. Make sure you've updated trunk to the latest rev before running<br>
> update_llc_test_checks.py on lea-3.ll. Ie, I would only expect the<br>
> output you're seeing if you're running the script on a version of that<br>
> test file before r313631. After that commit, each RUN has its own check<br>
> prefix, so there should be no conflict opportunity.<br>
><br>
> 2. I didn't realize the scope of the patch covered all targets and both<br>
> scalars and vectors. That isn't going to work as-is. We can't assume<br>
> that every target and data type will prefer to replace that multiply.<br>
> Even the x86 diffs in lea-3.ll are regressions:<br>
><br>
> -; LNX1-NEXT: leal (%rdi,%rdi,2), %eax<br>
> +; LNX1-NEXT: leal (,%rdi,4), %eax<br>
> +; LNX1-NEXT: subl %edi, %eax<br>
><br>
> I suggest taking a smaller first step by limiting the patch to cases<br>
> where the multiply is not a legal op for a target (TLI.isOperationLegal()).<br>
><br>
> 3. Since the patch can cause a crash, that needs to be investigated<br>
> first. I didn't look into it, but my guess for the Hexagon crash (and<br>
> probably other targets) is that you're trying to create illegal<br>
> ops after the DAG has been legalized. So that's another potential way to<br>
> limit the scope of the patch - don't try the transform unless we're<br>
> pre-legalization:<br>
> if (Level < AfterLegalizeDAG) { // do something }<br>
><br>
><br>
><br>
> On Wed, Sep 20, 2017 at 4:17 AM, Haidl, Michael<br>
> <<a href="mailto:michael.haidl@uni-muenster.de">michael.haidl@uni-muenster.de</a> <mailto:<a href="mailto:michael.haidl@uni-muenster.de">michael.haidl@uni-<wbr>muenster.de</a>>><br>
> wrote:<br>
><br>
> Hi,<br>
><br>
> I am currently working on a more or less intrusive patch (D37896), which<br>
> pulls optimizations on multiplications from some back-ends, e.g., (mul<br>
> x, 2^N + 1) => (add (shl x, N), x) in AArch64, into the DAGCombiner to<br>
> have this optimization generic on all targets.<br>
> However, running the LLVM Tests leads to 67 unexpected results.<br>
><br>
> Am 19.09.2017 um 15:58 schrieb Sanjay Patel:<br>
> > For the tests that are changing, you should see if those changes are<br>
> > improvements, regressions, or neutral. This is unfortunately not<br>
> always<br>
> > obvious for x86 asm, so feel free to just post those diffs in an<br>
> updated<br>
> > version of the patch at D37896.<br>
> ><br>
> > If the test files have auto-generated assertions (look for this<br>
> string<br>
> > on the first line of the test file: "NOTE: Assertions have been<br>
> > autogenerated by utils/update_llc_test_checks.<wbr>py"...<br>
> > and both of these do as of: <a href="https://reviews.llvm.org/rL313631" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>rL313631</a><br>
> <<a href="https://reviews.llvm.org/rL313631" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>rL313631</a>><br>
> > <<a href="https://reviews.llvm.org/rL313631" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>rL313631</a><br>
> <<a href="https://reviews.llvm.org/rL313631" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>rL313631</a>>> ), then it's easy to observe the<br>
> > diffs by re-running that script after your code patch is applied:<br>
> > $ /path/to/update_llc_test_<wbr>checks.py --llc=/path/to/local/and/new/<wbr>llc<br>
> > lea-3.ll<br>
> > $ svn diff lea-3.ll<br>
><br>
> As described by Sanjay in the original thread, I updated the llc tests<br>
> but some of the tests were not updated.<br>
><br>
> For example: CodeGen/X86/lea-3.ll produces the following output:<br>
> llvm/utils/update_llc_test_<wbr>checks.py --llc=/home/dev/local/bin/llc<br>
> CodeGen/X86/lea-3.ll<br>
> WARNING: Found conflicting asm under the same prefix: 'CHECK'!<br>
> WARNING: Found conflicting asm under the same prefix: 'CHECK'!<br>
> WARNING: Found conflicting asm under the same prefix: 'CHECK'!<br>
><br>
> leaving parts of the test unchanged and the tests still fail.<br>
><br>
> Other tests like CodeGen/AArch64/aarch64-gep-<wbr>opt.ll results in:<br>
> WARNING: Skipping non-FileChecked RUN line: llc -O3<br>
> -aarch64-enable-gep-opt=true -mattr=-use-aa -print-after=codegenprepare<br>
> < %s >%t 2>&1 && FileCheck --check-prefix=CHECK-NoAA <%t %s<br>
> WARNING: Skipping non-FileChecked RUN line: llc -O3<br>
> -aarch64-enable-gep-opt=true -mattr=+use-aa -print-after=codegenprepare<br>
> < %s >%t 2>&1 && FileCheck --check-prefix=CHECK-UseAA <%t %s<br>
> WARNING: Skipping non-FileChecked RUN line: llc -O3<br>
> -aarch64-enable-gep-opt=true -print-after=codegenprepare -mcpu=cyclone <<br>
> %s >%t 2>&1 && FileCheck --check-prefix=CHECK-NoAA <%t %s<br>
> WARNING: Skipping non-FileChecked RUN line: llc -O3<br>
> -aarch64-enable-gep-opt=true -print-after=codegenprepare<br>
> -mcpu=cortex-a53 < %s >%t 2>&1 && FileCheck --check-prefix=CHECK-UseAA<br>
> <%t %s<br>
><br>
> And the worst thing I witnessed was with the Hexagon back-end were my<br>
> changes in DAGCombiner trigger an Unreachable:<br>
><br>
> home/dev/llvm/build/./bin/llc -march=hexagon -mcpu=hexagonv5<br>
> -disable-hsdr <<br>
> /home/dev/llvm/llvm/test/<wbr>CodeGen/Hexagon/vect/vect-cst-<wbr>v4i32.ll |<br>
> /home/dev/llvm/build/./bin/<wbr>FileCheck<br>
> /home/dev/llvm/llvm/test/<wbr>CodeGen/Hexagon/vect/vect-cst-<wbr>v4i32.ll<br>
> --<br>
> Exit Code: 2<br>
><br>
> Command Output (stderr):<br>
> --<br>
> ReplaceNodeResults not implemented for this target!<br>
> UNREACHABLE executed at<br>
> /home/dev/llvm/llvm/include/<wbr>llvm/Target/TargetLowering.h:<wbr>3164!<br>
> #0 0x00007f1b2d330cfa llvm::sys::PrintStackTrace(<wbr>llvm::raw_ostream&)<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMSupport.so.6+<wbr>0x116cfa)<br>
> #1 0x00007f1b2d32ea9e llvm::sys::RunSignalHandlers()<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMSupport.so.6+<wbr>0x114a9e)<br>
> #2 0x00007f1b2d32ec04 SignalHandler(int)<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMSupport.so.6+<wbr>0x114c04)<br>
> #3 0x00007f1b2c3ed7f0 (/lib/x86_64-linux-gnu/libc.<wbr>so.6+0x357f0)<br>
> #4 0x00007f1b2c3ed77f gsignal (/lib/x86_64-linux-gnu/libc.<wbr>so.6+0x3577f)<br>
> #5 0x00007f1b2c3ef37a abort (/lib/x86_64-linux-gnu/libc.<wbr>so.6+0x3737a)<br>
> #6 0x00007f1b2d2b012a<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMSupport.so.6+<wbr>0x9612a)<br>
> #7 0x00007f1b359326ec<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMHexagonCodeGen.so.<wbr>6+0x1176ec)<br>
> #8 0x00007f1b2d6a37a6<br>
> llvm::DAGTypeLegalizer::<wbr>CustomLowerNode(llvm::SDNode*, llvm::EVT, bool)<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMSelectionDAG.so.6+<wbr>0x1247a6)<br>
> #9 0x00007f1b2d6be0d9<br>
> llvm::DAGTypeLegalizer::<wbr>SplitVectorResult(llvm::<wbr>SDNode*, unsigned int)<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMSelectionDAG.so.6+<wbr>0x13f0d9)<br>
> #10 0x00007f1b2d69f156 llvm::DAGTypeLegalizer::run()<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMSelectionDAG.so.6+<wbr>0x120156)<br>
> #11 0x00007f1b2d6a01ff llvm::SelectionDAG::<wbr>LegalizeTypes()<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMSelectionDAG.so.6+<wbr>0x1211ff)<br>
> #12 0x00007f1b2d785d42 llvm::SelectionDAGISel::<wbr>CodeGenAndEmitDAG()<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMSelectionDAG.so.6+<wbr>0x206d42)<br>
> #13 0x00007f1b2d790fd8<br>
> llvm::SelectionDAGISel::<wbr>SelectAllBasicBlocks(llvm::<wbr>Function const&)<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMSelectionDAG.so.6+<wbr>0x211fd8)<br>
> #14 0x00007f1b2d7930e2<br>
> llvm::SelectionDAGISel::<wbr>runOnMachineFunction(llvm::<wbr>MachineFunction&)<br>
> [clone .part.873]<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMSelectionDAG.so.6+<wbr>0x2140e2)<br>
> #15 0x00007f1b35931d6a (anonymous<br>
> namespace)::<wbr>HexagonDAGToDAGISel::<wbr>runOnMachineFunction(llvm::<wbr>MachineFunction&)<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMHexagonCodeGen.so.<wbr>6+0x116d6a)<br>
> #16 0x00007f1b2ef22585<br>
> llvm::MachineFunctionPass::<wbr>runOnFunction(llvm::Function&)<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMCodeGen.so.6+<wbr>0x21f585)<br>
> #17 0x00007f1b2e922dd3<br>
> llvm::FPPassManager::<wbr>runOnFunction(llvm::Function&)<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMCore.so.6+0x18ddd3)<br>
> #18 0x00007f1b2e922e93 llvm::FPPassManager::<wbr>runOnModule(llvm::Module&)<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMCore.so.6+0x18de93)<br>
> #19 0x00007f1b2e9239e1 llvm::legacy::PassManagerImpl:<wbr>:run(llvm::Module&)<br>
> (/home/dev/llvm/build/bin/../<wbr>lib/libLLVMCore.so.6+0x18e9e1)<br>
> #20 0x000055b29c4ef3e2 compileModule(char**, llvm::LLVMContext&)<br>
> (/home/dev/llvm/build/./bin/<wbr>llc+0x213e2)<br>
> #21 0x000055b29c4e23bc main (/home/dev/llvm/build/./bin/<wbr>llc+0x143bc)<br>
> #22 0x00007f1b2c3d83f1 __libc_start_main<br>
> (/lib/x86_64-linux-gnu/libc.<wbr>so.6+0x203f1)<br>
> #23 0x000055b29c4e256a _start (/home/dev/llvm/build/./bin/<wbr>llc+0x1456a)<br>
> Stack dump:<br>
> 0. Program arguments: /home/dev/llvm/build/./bin/llc -march=hexagon<br>
> -mcpu=hexagonv5 -disable-hsdr<br>
> 1. Running pass 'Function Pass Manager' on module '<stdin>'.<br>
> 2. Running pass 'Hexagon DAG->DAG Pattern Instruction Selection' on<br>
> function '@run'<br>
> FileCheck error: '-' is empty.<br>
> FileCheck command line: /home/dev/llvm/build/./bin/<wbr>FileCheck<br>
> /home/dev/llvm/llvm/test/<wbr>CodeGen/Hexagon/vect/vect-cst-<wbr>v4i32.ll<br>
><br>
> How do I proceed to get the patch running all tests as expected? The<br>
> Tests directly focused on the results of the patch are passing without<br>
> problems.<br>
><br>
> Cheers,<br>
> Michael<br>
><br>
><br>
</blockquote></div><br></div>