[llvm-dev] [Hexagon] Type Legalization

Haidl, Michael via llvm-dev llvm-dev at lists.llvm.org
Thu Sep 21 22:06:03 PDT 2017


Hi Sanjay,

thanks for this information. I did get a little bit further with the 
patch. However, Hexagon gives me headaches.

I tried to limit the scope of the patch to the BeforeLegalizeTypes phase 
and Hexagon still reaches the unreachable. Hexagon tries to split or 
widen a vector type for a node with custom lowering where the 
unreachable arises from inside TargetLowering::ReplaceNodeResults which 
is not overwritten in the Hexagon backend.

To avoid generating illegal code after all I queried TLI if all 
generated operations are legal operations for the given vector type.

   if (   TLI.isOperationLegal(ISD::SUB, VT)
       && TLI.isOperationLegal(ISD::ADD, VT)
       && TLI.isOperationLegal(ISD::SHL, VT)
       && TLI.isOperationLegal(ISD::SRA, VT)) {

The Hexagon backend says happily: yes! Go ahead!
When it comes to HexagonDAGToDAGISel and the type should be legalized no 
custom lowering is found and TargetLowering::ReplaceNodeResults is 
called which falls back to the default implementation and the 
unreachable is trigged.

Is this really working as intended or am I missing something here?

Cheers,
Michael

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


More information about the llvm-dev mailing list