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> 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>> > > 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 > > > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170921/99b4244e/attachment.html>
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
> >
> >
>
>
Ok I double checked it and TLI.isTypeLegal does work. My setup was messed up and an earlier version of the *.so was loaded. Cheers, Michael On 22.09.2017 07:46, Haidl, Michael via llvm-dev wrote:> 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 >> > >> > >> >> > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >