rL306379
authorLLVM Packaging Team <pkg-llvm-team@lists.alioth.debian.org>
Mon, 2 Jul 2018 08:45:18 +0000 (09:45 +0100)
committerJulien Cristau <jcristau@mozilla.com>
Mon, 2 Jul 2018 08:45:18 +0000 (09:45 +0100)
commit652fa56689666bab03e2ddb767b12f8729660dc9
treecf52591de7a26e644a02645171b9b4659169341f
parentfd18345100e85e8374be3b8a304d20a6d3e81f1b
rL306379

commit 7df06519765b14e1b08d7034c82c45a0a653eb25
Author: Chandler Carruth <chandlerc@gmail.com>
Date:   Tue Jun 27 08:32:03 2017 +0000

    [SROA] Fix PR32902 by more carefully propagating !nonnull metadata.

    This is based heavily on the work done ni D34285. I mostly wanted to do
    test cleanup for the author to save them some time, but I had a really
    hard time understanding why it was so hard to write better test cases
    for these issues.

    The problem is that because SROA does a second rewrite of the loads and
    because we *don't* propagate !nonnull for non-pointer loads, we first
    introduced invalid !nonnull metadata and then stripped it back off just
    in time to avoid most ways of this PR manifesting. Moving to the more
    careful utility only fixes this by changing the predicate to look at the
    new load's type rather than the target type. However, that *does* fix
    the bug, and the utility is much nicer including adding range metadata
    to model the nonnull property after a conversion to an integer.

    However, we have bigger problems because we don't actually propagate
    *range* metadata, and the utility to do this extracted from instcombine
    isn't really in good shape to do this currently. It *only* handles the
    case of copying range metadata from an integer load to a pointer load.
    It doesn't even handle the trivial cases of propagating from one integer
    load to another when they are the same width! This utility will need to
    be beefed up prior to using in this location to get the metadata to
    fully survive.

    And even then, we need to go and teach things to turn the range metadata
    into an assume the way we do with nonnull so that when we *promote* an
    integer we don't lose the information.

    All of this will require a new test case that looks kind-of like
    `preserve-nonnull.ll` does here but focuses on range metadata. It will
    also likely require more testing because it needs to correctly handle
    changes to the integer width, especially as SROA actively tries to
    change the integer width!

    Last but not least, I'm a little worried about hooking the range
    metadata up here because the instcombine logic for converting from
    a range metadata *to* a nonnull metadata node seems broken in the face
    of non-zero address spaces where null is not mapped to the integer `0`.
    So that probably needs to get fixed with test cases both in SROA and in
    instcombine to cover it.

    But this *does* extract the core PR fix from D34285 of preventing the
    !nonnull metadata from being propagated in a broken state just long
    enough to feed into promotion and crash value tracking.

    On D34285 there is some discussion of zero-extend handling because it
    isn't necessary. First, the new load size covers all of the non-undef
    (ie, possibly initialized) bits. This may even extend past the original
    alloca if loading those bits could produce valid data. The only way its
    valid for us to zero-extend an integer load in SROA is if the original
    code had a zero extend or those bits were undef. And we get to assume
    things like undef *never* satifies nonnull, so non undef bits can
    participate here. No need to special case the zero-extend handling, it
    just falls out correctly.

    The original credit goes to Ariel Ben-Yehuda! I'm mostly landing this to
    save a few rounds of trivial edits fixing style issues and test case
    formulation.

    Differental Revision: D34285

    git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@306379 91177308-0d34-0410-b5e6-96231b3b80d8

Gbp-Pq: Name rL306379.diff
lib/Transforms/Scalar/SROA.cpp
test/Transforms/SROA/preserve-nonnull.ll