Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove implicit sign extension assumptions from iRegStore evaluator (0.15.0) #30

Merged
merged 1 commit into from
Jul 10, 2019
Merged

Remove implicit sign extension assumptions from iRegStore evaluator (0.15.0) #30

merged 1 commit into from
Jul 10, 2019

Commits on Jul 10, 2019

  1. Remove implicit sign extension assumptions from iRegStore evaluator 

    The `TR::iRegStore` evaluator on Z makes an implicit assumption that if
the value being globally stored is a conversion of an appropriate type,
for example `TR::su2i` of a `TR::cloadi` then the evaluators will have
generated a full 64-bit sign extended load.

This is very dangerous as an evaluator should never implicitly assume
another evaluator will generate a specific instruction sequence to
perform some action. Because of this implicit assumption the
`TR::iRegStore` evaluator omitted the generation of an explicit sign
extension and just assumed the evaluator has already done it. It then
marked the register via the `setAlreadySignExtended` API that the
extension has been performed. Later a `TR::i2l` conversion operation
consumes the value of the `TR::su2i` and it checks whether the child
register has been marked via the `alreadySignExtended` API in which
case it would return true, however no sign extension was performed.

This ends up being a functional problem as we could have garbage values
in the high-order 32-bits which were never cleared.

Rather than allowing implicit assumptions about sign extensions we
leave that to the LoadExtensions optimization and perform the sign
extension explicitly via an `LGFR` instruction.

An example of the trees encountered is the following:

```
------------------------------
 n4469n   (  0)  iRegStore GPR7  (Unsigned NeedsSignExt privatizedInlinerArg )
 n4470n   (  4)    su2i (in GPR_1545) (X>=0 unneededConv )
 n4471n   (  0)      cloadi  <array-shadow>[#236  Shadow] [flags 0x80000602 0x0 ] (in GPR_1545) (cannotOverflow zeroExtendTo32BitAtSource )
 n14613n  (  1)        ==>aRegLoad (in *GPR_1537) (X!=0 X>=0 SeenRealReference sharedMemory )
------------------------------

 [     0x3ff1a2e9790]   LLH     GPR_1545, Shadow[<array-shadow>] 0(*GPR_1537)

...
...

------------------------------
 n2929n   (  0)  ificmplt --> block_1286 BBStart at n13709n ()
 n2923n   (  2)    b2i (in GPR_1554) (unneededConv )
 n2922n   (  0)      bloadi  <array-shadow>[#235  Shadow] [flags 0x80000601 0x0 ] (in GPR_1554) (cannotOverflow signExtendedTo32BitAtSource )
 n14616n  (  3)          ==>aRegLoad (in &GPR_1540) (X!=0 SeenRealReference sharedMemory )
 n2920n   (  0)          lsub (in GPR_1545) (highWordZero X>=0 cannotOverflow )
 n2919n   (  0)            i2l (in GPR_1545) (highWordZero X>=0 )
 n4470n   (  0)              ==>su2i (in GPR_1545) (X>=0 unneededConv )
 n2918n   (  0)            lconst -8 (X!=0 X<=0 )
 n2926n   (  0)    iconst 0 (X==0 X>=0 X<=0 )
 n14638n  (  0)    GlRegDeps ()
 n14612n  (  3)      ==>aRegLoad (in &GPR_1536)
 n14614n  (  2)      ==>iRegLoad (in GPR_1538) (X>=0 cannotOverflow SeenRealReference )
 n14616n  (  3)      ==>aRegLoad (in &GPR_1540) (X!=0 SeenRealReference sharedMemory )
------------------------------

 [     0x3ff1a2eafd0]   LB      GPR_1554, Shadow[<array-shadow>] 8(GPR_1545,&GPR_1540)
```

Fixes: eclipse-openj9/openj9#6248
Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>
    @fjeremic
    fjeremic committed Jul 10, 2019
    Configuration menu
    Copy the full SHA
    23933aa View commit details
    Browse the repository at this point in the history