[llvm-dev] [Hexagon] Type Legalization

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


Hi Craig,

protecting the transformation with:
   if (TLI.isTypeLegal(VT)
       && TLI.isOperationLegal(ISD::SUB, VT)
       && TLI.isOperationLegal(ISD::ADD, VT)
       && TLI.isOperationLegal(ISD::SHL, VT)
       && TLI.isOperationLegal(ISD::SRA, VT)) {

shows the same result.

Michael

On 22.09.2017 07:19, Craig Topper wrote:
> 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.
> 
> ~Craig
> 
> On Thu, Sep 21, 2017 at 10:06 PM, Haidl, Michael 
> <michael.haidl at uni-muenster.de <mailto:michael.haidl at uni-muenster.de>> 
> wrote:
> 
>     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>
>     <mailto: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>>
>      >      > <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