This is an archive of the discontinued LLVM Phabricator instance.

[X86] Remove dead nodes left after ReplaceAllUsesWith calls during address matching
ClosedPublic

Authored by craig.topper on Apr 23 2019, 5:58 PM.

Details

Summary

ReplaceAllUsesWith doesn't remove the node that was replaced. So its left around in the graph messing up use counts on other nodes.

One thing to note, is that this isn't valid if the node being deleted is the root node of an LEA match that gets rejected. In that case the node needs to stay alive because the isel table walking code would still have a reference to it that its going to try to match next. I don't think that's the case here though because the nodes being deleted here should be "and", "srl", and "zero_extend" none of which can be the root node of an LEA match.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Apr 23 2019, 5:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 23 2019, 5:58 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
craig.topper marked 2 inline comments as done.Apr 23 2019, 6:03 PM
craig.topper added inline comments.
llvm/test/CodeGen/X86/fold-and-shift.ll
9 ↗(On Diff #196360)

Looks like we were previously selecting movzx because the load folding was being suppressed by an artificial extra use on the load. Now we favor folding the load over folding the immediate like we normally would. We can probably hack IsProfitableToFold to restore the old behavior. The best option would be to narrow the load and use movzx from memory. But that's only valid if the load isn't volatile. I suppose we probably want the previous behavior in the case of a volatile load?

26 ↗(On Diff #196360)

Similar to above

niravd accepted this revision.Apr 24 2019, 8:55 AM

LGTM.

DAGCombiner now has infrastructure to strip away nodes that were created but unused in during a visit. It probably makes sense to do something similar in ISel

This revision is now accepted and ready to land.Apr 24 2019, 8:55 AM
This revision was automatically updated to reflect the committed changes.
hctim added a subscriber: hctim.Apr 24 2019, 2:16 PM

Looks like this change introduced a use-after-poison (aka use-after-d'tor) (asan buildbot).

PTAL :)

==6559==ERROR: AddressSanitizer: use-after-poison on address 0x62100003244a at pc 0x00000305b716 bp 0x7ffeac52afb0 sp 0x7ffeac52afa8
READ of size 2 at 0x62100003244a thread T0
    #0 0x305b715 in getValueType /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/CodeGen/SelectionDAGNodes.h:989:5
    #1 0x305b715 in getValueType /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/CodeGen/SelectionDAGNodes.h:1137
    #2 0x305b715 in getSimpleValueType /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/CodeGen/SelectionDAGNodes.h:169
    #3 0x305b715 in (anonymous namespace)::X86DAGToDAGISel::selectAddr(llvm::SDNode*, llvm::SDValue, llvm::SDValue&, llvm::SDValue&, llvm::SDValue&, llvm::SDValue&, llvm::SDValue&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:2063
    #4 0x3047a07 in (anonymous namespace)::X86DAGToDAGISel::CheckComplexPattern(llvm::SDNode*, llvm::SDNode*, llvm::SDValue, unsigned int, llvm::SmallVectorImpl<std::__1::pair<llvm::SDValue, llvm::SDNode*> >&) /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/lib/Target/X86/X86GenDAGISel.inc:275229:10
    #5 0x5b56325 in llvm::SelectionDAGISel::SelectCodeCommon(llvm::SDNode*, unsigned char const*, unsigned int) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:3275:12
    #6 0x302eafe in (anonymous namespace)::X86DAGToDAGISel::Select(llvm::SDNode*) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
    #7 0x5b3bed8 in llvm::SelectionDAGISel::DoInstructionSelection() /b/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1139:7
    #8 0x5b37154 in llvm::SelectionDAGISel::CodeGenAndEmitDAG() /b/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:937:5
    #9 0x5b2dd26 in llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1814:7
    #10 0x5b22d25 in llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:496:3
    #11 0x301ac1c in (anonymous namespace)::X86DAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:191:25
    #12 0x41a85aa in llvm::MachineFunctionPass::runOnFunction(llvm::Function&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/MachineFunctionPass.cpp:73:13
    #13 0x4c312d3 in llvm::FPPassManager::runOnFunction(llvm::Function&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1648:27
    #14 0x4c31b52 in llvm::FPPassManager::runOnModule(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1685:16
    #15 0x4c32a1c in runOnModule /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1752:27
    #16 0x4c32a1c in llvm::legacy::PassManagerImpl::run(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1865
    #17 0xa6a209 in compileModule(char**, llvm::LLVMContext&) /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llc/llc.cpp:609:8
    #18 0xa63d50 in main /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llc/llc.cpp:363:22
    #19 0x7f585706f2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #20 0x9565d9 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/llc+0x9565d9)

0x62100003244a is located 842 bytes inside of 4096-byte region [0x621000032100,0x621000033100)
allocated by thread T0 here:
    #0 0xa227ff in malloc /b/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146
    #1 0xaa1143 in safe_malloc /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/MemAlloc.h:26:18
    #2 0xaa1143 in Allocate /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/Allocator.h:99
    #3 0xaa1143 in llvm::BumpPtrAllocatorImpl<llvm::MallocAllocator, 4096ul, 4096ul>::StartNewSlab() /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/Allocator.h:400
    #4 0xaa0e79 in llvm::BumpPtrAllocatorImpl<llvm::MallocAllocator, 4096ul, 4096ul>::Allocate(unsigned long, unsigned long) /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/Allocator.h:260:5
    #5 0x5a44138 in Allocate<llvm::FrameIndexSDNode, llvm::BumpPtrAllocatorImpl<llvm::MallocAllocator, 4096, 4096> > /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/Recycler.h:89:57
    #6 0x5a44138 in Allocate<llvm::FrameIndexSDNode> /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/RecyclingAllocator.h:43
    #7 0x5a44138 in newSDNode<llvm::FrameIndexSDNode, int &, llvm::EVT &, bool &> /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/CodeGen/SelectionDAG.h:341
    #8 0x5a44138 in llvm::SelectionDAG::getFrameIndex(int, llvm::EVT, bool) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:1389
    #9 0x30986e0 in llvm::X86TargetLowering::LowerMemArgument(llvm::SDValue, unsigned int, llvm::SmallVectorImpl<llvm::ISD::InputArg> const&, llvm::SDLoc const&, llvm::SelectionDAG&, llvm::CCValAssign const&, llvm::MachineFrameInfo&, unsigned int) const /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/X86ISelLowering.cpp:3068:21
    #10 0x3099f9f in llvm::X86TargetLowering::LowerFormalArguments(llvm::SDValue, unsigned int, bool, llvm::SmallVectorImpl<llvm::ISD::InputArg> const&, llvm::SDLoc const&, llvm::SelectionDAG&, llvm::SmallVectorImpl<llvm::SDValue>&) const /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/X86ISelLowering.cpp:3266:11
    #11 0x59d919a in llvm::SelectionDAGISel::LowerArguments(llvm::Function const&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:9503:26
    #12 0x5b29480 in llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1590:5
    #13 0x5b22d25 in llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:496:3
    #14 0x301ac1c in (anonymous namespace)::X86DAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:191:25
    #15 0x41a85aa in llvm::MachineFunctionPass::runOnFunction(llvm::Function&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/CodeGen/MachineFunctionPass.cpp:73:13
    #16 0x4c312d3 in llvm::FPPassManager::runOnFunction(llvm::Function&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1648:27
    #17 0x4c31b52 in llvm::FPPassManager::runOnModule(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1685:16
    #18 0x4c32a1c in runOnModule /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1752:27
    #19 0x4c32a1c in llvm::legacy::PassManagerImpl::run(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1865
    #20 0xa6a209 in compileModule(char**, llvm::LLVMContext&) /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llc/llc.cpp:609:8
    #21 0xa63d50 in main /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llc/llc.cpp:363:22
    #22 0x7f585706f2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)

SUMMARY: AddressSanitizer: use-after-poison /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/CodeGen/SelectionDAGNodes.h:989:5 in getValueType
Shadow bytes around the buggy address:
  0x0c427fffe430: 00 00 00 00 00 00 00 00 00 00 00 f7 00 00 00 00
  0x0c427fffe440: 00 00 00 00 00 00 00 00 00 f7 00 00 00 00 00 00
  0x0c427fffe450: 00 00 00 00 00 00 00 f7 f7 f7 f7 02 f7 f7 f7 f7
  0x0c427fffe460: f7 f7 f7 f7 f7 f7 00 00 00 00 00 00 00 00 00 00
  0x0c427fffe470: 00 00 00 f7 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c427fffe480: 00 f7 f7 f7 f7 02 f7 f7 f7[f7]f7 f7 f7 f7 f7 f7
  0x0c427fffe490: f7 f7 f7 02 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
  0x0c427fffe4a0: f7 02 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 00 00 00 00
  0x0c427fffe4b0: 00 00 00 00 00 00 00 00 00 f7 00 00 00 00 00 00
  0x0c427fffe4c0: 00 00 00 00 00 00 00 f7 00 00 00 00 00 00 00 00
  0x0c427fffe4d0: 00 00 00 00 00 f7 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==6559==ABORTING