#25 IR Verification Patches

closed
Dave Grove
None
5
2012-09-21
2005-09-21
Ian Rogers
No

IR verification has shown a number of problems with
compiler phases as listed with bug #1274081. This is
intended to be a series of patches to fix this
culminating in the improved IR verification, part of
which will become part of the development build (and
all of it if IR.PARANOID is true).

    STATEMENT OF ORIGIN FOR A SINGLE CONTRIBUTOR

I, Ian Rogers:

(a) represent that either:

(i) I am the only author and owner of the contributed
software
(described as/entitled IR Verification Patches),
which was neither derived nor copied from any
other software,
or

(ii) that any exception to (i) is software which was
obtained under the
CPL (Common Public License),

and

(b) hereby agree to license this contributed software
under the CPL.

This first patch is for OPT_DynamicTypeCheck including
a partial re-write of the object array store check. In
checking this patch the error reporting from the BURS
phase was improved slightly (to make runtime BURS
failures intelligible).

The patch has been checked on Intel and PowerPC,
however, when running SpecJVM's raytrace and mtrt with
the "-m50 -M50 -a" flags failure ensues. This is,
however, the behaviour I'm observing without the object
array store modification.

Discussion

1 2 3 > >> (Page 1 of 3)
  • Ian Rogers
    Ian Rogers
    2005-09-21

    Patch against current CVS head containing differences in OPT_DynamicTypeCheck and OPT_ConvertLIRtoMIR

     
  • Dave Grove
    Dave Grove
    2005-09-22

    Logged In: YES
    user_id=1215435

    Changes to array store check expansion cause large scale
    regressions.

     
  • Ian Rogers
    Ian Rogers
    2005-09-22

    Logged In: YES
    user_id=308843

    The rest of the verification fixes excluding array store
    check and without the IR verifier.

     
  • Ian Rogers
    Ian Rogers
    2005-09-22

    Fixes excluding array store check expansion

     
    Attachments
  • Ian Rogers
    Ian Rogers
    2005-09-22

    Logged In: YES
    user_id=308843

    To summarize the changes in the patch:
    - added more use of Operand.copy() to avoid sharing
    - remove Operand.clear() as its not need when operand
    sharing is removed as a bug
    - add missing out edges in final MIR expansion
    - add compact node numbering when a compiler phase removes a
    node from the CFG
    - clean up OPT_Instruction so it behaves as its described
    - make loop unrolling invariant and get invariant functions
    succeed in the same circumstances (i.e. remove case of
    invariant results being copied to before where they're defined)
    - make redundant branch optimization behave itself with
    guards of ifcmps its removing
    - stop simplifier from simplifying things to traps unless
    they are at the end of basic blocks

     
  • Dave Grove
    Dave Grove
    2005-09-22

    Logged In: YES
    user_id=1215435

    grabbed it and started testing. Glanced through it briefly
    and I'm likely to drop a couple of the changes before
    committing it. In particular, the "right" way to fix
    OPT_Simplifier is to split the basic block, not to disable
    the optimization. Can clear that up later...

     
  • Dave Grove
    Dave Grove
    2005-09-23

    Logged In: YES
    user_id=1215435

    second patch is also failing some regression tests.

    Best course is probably to break into smaller units of work.

     
  • Ian Rogers
    Ian Rogers
    2005-09-23

    Logged In: YES
    user_id=308843

    That's a shame. I'll break it up further.

    The simplifier stuff is more complex than that even, as when
    instructions are folded we don't remove instructions
    dependent on their results. In the case of trap folding this
    means that IR starts to float around that's using a
    temporary that's never defined. I don't think the simplifier
    should break blocks as when something is trap folded the
    instructions following are dead code (how can we reach them
    if we definitely trap?). I didn't want to change this
    behaviour though as I was worried what effect might be had
    on the iterators using OPT_Simplifier (would they take
    kindly to instructions vanishing?). Anyway, it needs a tidy
    up and like you say it can be done later.

    I'm going to try the CVS head here again. I'm not sure if
    the regressions are working with the CVS head. I thought the
    last patch was relatively tame, but to start getting it
    committed I will try to split it up further. May be we can
    commit the verifier first (but have it disabled unless
    PARANOID is set) then commit the verifier fixes. This way
    you can better see what's going on? I'll do this as a patch
    first.

    Ian

     
  • Ian Rogers
    Ian Rogers
    2005-09-23

    Logged In: YES
    user_id=308843

    I don't like the changes to loop unrolling either, but when
    I can get the verifier stuff in I will send a patch for our
    improved loop unrolling stuff. What I wanted was to get the
    IR into a state where it may not be as optimal as before but
    the verifier was happy with it. Then I would submit patches
    to get beyond the performance we previously had - namely
    through loop optimisations to null/bound checks and improved
    loop unrolling. But without the verifier and a number of the
    fixes I know that the loop unrolling wouldn't work (house
    built on sinking sand, that kind of idea..).

    Ian

     
  • Dave Grove
    Dave Grove
    2005-09-23

    Logged In: YES
    user_id=1215435

    On the off chance it's useful, here's a summary of what
    failed with the second patch. I didn't have a chance to see
    exactly what was going wrong on any of them, but spot
    checked a few and the failures mostly seem to be "real"

    RunSanityTests: prototype-opt: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/gctest
    RunSanityTests: prototype-opt: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/SPECjvm98
    RunSanityTests: prototype-opt: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/SPECjbb2000
    RunSanityTests: development: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/reflect
    RunSanityTests: development: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/xalan
    RunSanityTests: development: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/SPECjvm98
    RunSanityTests: development: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/SPECjbb2000
    RunSanityTests: production: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/xalan
    RunSanityTests: production: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/SPECjvm98
    RunSanityTests: production: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/SPECjbb2000
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/gctest
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/bytecodeTests
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/opttests
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/reflect
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/threads
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/jni
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/CaffeineMark
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/jBYTEmark
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/javalex
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/xerces
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/xalan
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/soot
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/ipsixql
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/SPECjvm98
    RunSanityTests: FullAdaptiveCopyMS: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/SPECjbb2000
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/gctest
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/bytecodeTests
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/opttests
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/reflect
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/threads
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/jni
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/CaffeineMark
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/jBYTEmark
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/javalex
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/xerces
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/xalan
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/soot
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/ipsixql
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/SPECjvm98
    RunSanityTests: FullAdaptiveMarkSweep: You are NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/SPECjbb2000
    RunSanityTests: ExtremeAssertionsOptAdaptiveCopyMS: You are
    NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/gctest
    RunSanityTests: ExtremeAssertionsOptAdaptiveCopyMS: You are
    NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/bytecodeTests
    RunSanityTests: ExtremeAssertionsOptAdaptiveCopyMS: You are
    NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/opttests
    RunSanityTests: ExtremeAssertionsOptAdaptiveCopyMS: You are
    NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/reflect
    RunSanityTests: ExtremeAssertionsOptAdaptiveCopyMS: You are
    NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/threads
    RunSanityTests: ExtremeAssertionsOptAdaptiveCopyMS: You are
    NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/CaffeineMark
    RunSanityTests: ExtremeAssertionsOptAdaptiveCopyMS: You are
    NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/jBYTEmark
    RunSanityTests: ExtremeAssertionsOptAdaptiveCopyMS: You are
    NOT sane in
    /homes/angmar/dgrove/rvmRoot/rvm/regression/tests/javalex
    [dgrove@angmar ~/SCRATCH]$

     
  • Ian Rogers
    Ian Rogers
    2005-09-23

    Logged In: YES
    user_id=308843

    Thanks, definitely looks very broken :-) I think if you look
    at the patch though most things in it are modest. I am
    reverting things to the old version one-by-one, but as I do
    I'm getting build errors. I'm definitely getting lots of
    insane behaviour. The only two things different in my world
    to the normal builds I've been used to is the classpath
    version (now 0.18) and the patch. I hope I can find the
    cause of the grievance soon. Sorry for the delay though, I'm
    trying to do a few things at once.

    Ian

     
  • Ian Rogers
    Ian Rogers
    2005-09-23

    Logged In: YES
    user_id=308843

    Sorry, I will respond to this as soon as possible but
    probably not today. I have tried taking out the most
    dangerous parts of the patch but I'm still observing
    unexpected behaviour (_205_raytrace runs forever). I'm just
    hoping the bugs aren't caused by bug work arounds elsewhere
    in the system. Only debugging is going to tell me this..
    Regards,

    Ian

     
  • Ian Rogers
    Ian Rogers
    2005-10-04

    Logged In: YES
    user_id=308843

    Sorry for the delay. I'm back looking at this. I hope I can
    at least get most of the .copy() parts of the bug fix into a
    working patch today.
    Ian

     
  • Ian Rogers
    Ian Rogers
    2005-10-04

    Logged In: YES
    user_id=308843

    I've isolated a few files as causing problems. I've checked
    this patch on PowerPC and Intel and it seems to be ok. It
    gets a lot of the verifier related issues resolved. I hope
    to fix the remaining ones with a new patch, create a patch
    for the verifier and then create a patch for the improved
    loop unrolling code. If you want you can delay putting the
    loop unrolling fix in until the improved loop unrolling code
    is ready. The current loop unrolling is unsafe but will be
    more optimal than the fixed version. The problem lies in the
    invariant check being different from the code to get the
    terminal value, arraylength operations are regarded as
    invariant but this causes their results to be used before
    definition.

    Ian

     
  • Ian Rogers
    Ian Rogers
    2005-10-05

    A small patch of just what look like trivial fixes

     
    Attachments
  • Ian Rogers
    Ian Rogers
    2005-10-05

    Logged In: YES
    user_id=308843

    Please can you try the new patch. I'm still having
    difficulty with larger patches. This patch is nothing more
    than .copy added to stop operand sharing and a couple of
    insertOuts that were missing in the FinalMIR expansion.

    Ian

     
  • Ian Rogers
    Ian Rogers
    2005-10-06

    Logged In: YES
    user_id=308843

    Ok, the new patch doesn't work for me. I think the fixes
    are exposing us again to problems in mtrt/raytrace. But all
    the patch is doing is adding .copy where I've found operands
    being shared. Is the patch wrong or is something else wrong?
    I think something else. I will mail the core mailing list to
    see if that prompts any suggestions.

    Thanks,

    Ian

     
  • Dave Grove
    Dave Grove
    2005-10-06

    Logged In: YES
    user_id=1215435

    Committed NewVerifierFixesPatch (with 1 minor change to
    MIR_Multiply to get the 2 pieces of the DU/specification to
    match).

     
  • Ian Rogers
    Ian Rogers
    2005-10-06

    Logged In: YES
    user_id=308843

    Definitely agree with the DU specification change. Did the
    patch pass the regressions for you? It seems my regression
    testing machines are causing me problems if so. I will
    create a new patch to resolve a couple more problems so we
    can incrementally move forward. Thanks,

    Ian

     
  • Ian Rogers
    Ian Rogers
    2005-10-10

    differences to BC2HIR and generation context

     
    Attachments
  • Ian Rogers
    Ian Rogers
    2005-10-10

    Logged In: YES
    user_id=308843

    This patch fixes a number of the verification errors due to
    BC2HIR. It seems ok when I test it here. There is a
    remaining issue with completeExceptionHandlers creating
    unreachable code/blocks.

     
  • Dave Grove
    Dave Grove
    2005-10-10

    Logged In: YES
    user_id=1215435

    Testing BC2HIR patch; will commit this afternoon if no
    problems surface.

     
  • Dave Grove
    Dave Grove
    2005-10-10

    Logged In: YES
    user_id=1215435

    Committed BC2HIR.diff

     
  • Ian Rogers
    Ian Rogers
    2005-10-10

    differences to RBE and SSA tune up

     
    Attachments
  • Ian Rogers
    Ian Rogers
    2005-10-10

    Logged In: YES
    user_id=308843

    The patch is for redundant branch elimination. If a branch
    is eliminated the guard needs to be maintained as a move of
    the dominating branchs guard. Also if a block is removed
    from the code we need to compact the node numbering, which
    means the dominators are no longer valid, so the SSA tune up
    needs to fix this.

     
1 2 3 > >> (Page 1 of 3)