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 >