Skip to content
This repository has been archived by the owner on Feb 5, 2019. It is now read-only.

Improve the NullCheckElimination pass #14

Closed
wants to merge 1 commit into from
Closed

Improve the NullCheckElimination pass #14

wants to merge 1 commit into from

Conversation

zwarich
Copy link

@zwarich zwarich commented Jun 29, 2014

Teach the NullCheckElimination pass about recurrences involving inbounds
GEPs. This allows the pass to optimize null checks from most uses of
single slice and vector iterators.

This still isn't sufficient to eliminate null checks from uses of
multiple iterators with zip().

I also had my editor set to convert 8 spaces to a tab from a previous
project, so I'm undoing that here.

@alexcrichton
Copy link
Member

r? @pcwalton


enum CmpPred {
CmpEq,
CmpNe,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know that trailing commas were legal in C++ like this. Huh.

@pcwalton
Copy link

Looks good modulo a few nits.

@pcwalton
Copy link

or rather, one nit.

Teach the NullCheckElimination pass about recurrences involving inbounds
GEPs. This allows the pass to optimize null checks from most uses of
single slice and vector iterators.

This still isn't sufficient to eliminate null checks from uses of
multiple iterators with zip().
@zwarich
Copy link
Author

zwarich commented Jul 10, 2014

I rebased to mention that the passes responsible for the canonicalization are InstCombine / InstSimplify. Regarding your second nit, just in case GitHub loses it in the rebase I had this comment:

I don't actually rely on this assumption in the if (isZeroConstant(Op1)) { line. There Op0 is a nonnull-or-poison value, and Op1 might be null in the comparison Op1 <cmp> Op2, but there is no question of whether Op0 might be a zero constant regardless of canonicalization, since it's a known nonnull-or-poison value.

@alexcrichton
Copy link
Member

Sorry this fell under the radar! Merged as https://github.com/rust-lang/llvm/tree/rust-llvm-2014-07-20.

In the future feel free to ping me about merging these.

alexcrichton pushed a commit that referenced this pull request Aug 2, 2018
This reverts commit r337504 while I investigate a TSan bot failure that
seems related:

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/26526

    #8 0x000055581f2895d8 (/b/sanitizer-x86_64-linux-autoconf/build/tsan_debug_build/bin/clang-7+0x1eb45d8)
    #9 0x000055581f294323 llvm::ConstantAggrKeyType<llvm::ConstantArray>::create(llvm::ArrayType*) const /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/ConstantsContext.h:409:0
    #10 0x000055581f294323 llvm::ConstantUniqueMap<llvm::ConstantArray>::create(llvm::ArrayType*, llvm::ConstantAggrKeyType<llvm::ConstantArray>, std::pair<unsigned int, std::pair<llvm::ArrayType*, llvm::ConstantAggrKeyType<llvm::ConstantArray> > >&) /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/ConstantsContext.h:635:0
    #11 0x000055581f294323 llvm::ConstantUniqueMap<llvm::ConstantArray>::getOrCreate(llvm::ArrayType*, llvm::ConstantAggrKeyType<llvm::ConstantArray>) /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/ConstantsContext.h:654:0
    #12 0x000055581f2944cb llvm::ConstantArray::get(llvm::ArrayType*, llvm::ArrayRef<llvm::Constant*>) /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/Constants.cpp:964:0
    #13 0x000055581fa27e19 llvm::SmallVectorBase::size() const /b/sanitizer-x86_64-linux-autoconf/build/llvm/include/llvm/ADT/SmallVector.h:53:0
    #14 0x000055581fa27e19 llvm::SmallVectorImpl<llvm::Constant*>::resize(unsigned long) /b/sanitizer-x86_64-linux-autoconf/build/llvm/include/llvm/ADT/SmallVector.h:347:0
    #15 0x000055581fa27e19 (anonymous namespace)::EmitArrayConstant(clang::CodeGen::CodeGenModule&, clang::ConstantArrayType const*, llvm::Type*, unsigned int, llvm::SmallVectorImpl<llvm::Constant*>&, llvm::Constant*) /b/sanitizer-x86_64-linux-autoconf/build/llvm/tools/clang/lib/CodeGen/CGExprConstant.cpp:669:0

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@337511 91177308-0d34-0410-b5e6-96231b3b80d8
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants