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> 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> ), 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/20170920/054182e9/attachment.html>
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
>
>
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>
Sorry the part "type for a node with custom lowering" should be "type for a node without custom lowering". Michael On 22.09.2017 07:06, Haidl, Michael via llvm-dev 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 >> >> > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >