On Tue, Jun 14, 2011 at 6:04 PM, Jeff Law <l...@redhat.com> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > > As I've noted in prior messages; I'm looking to improve our path > isolation to improve code generation and reduce false positives from > warnings. > > The patch that's been in my queue for some time now (and I suspect it's > the final patch to our current implementation of jump threading) is > capable of isolating more paths, but is certainly not capable of fully > isolating every optimizable path through the CFG and eliminating all > unexecutable paths through the CFG (neither of which is actually > desirable due to potential code bloat issues). > > As a result of this better, but not full isolation, we can end up > exposing a constant propagation in a unexecutable path through the CFG > that isn't detected as unexecutable. As a result of exposing the > constant propagation we can trigger a bogus warning from -Warray-bounds. > > The problem is we might have something like this: > >> # BLOCK 11 freq:4946 >> # PRED: 9 [50.0%] (false,exec) 10 [100.0%] (fallthru,exec) 8 [28.0%] >> (false,exec) >> Invalid sum of incoming frequencies 2819, should be 4946 >> # D.39048_1 = PHI <3(9), D.39048_19(10), 4294967295(8)> >> # VUSE <.MEM_38(D)> >> D.39016_24 = default_target_hard_regs.x_fixed_regs[D.39048_1]; >> > > > - -Warray-bounds won't warn for this as it only triggers when we propagate > a constant for an array index and the constant is out of bounds of the > array. In this case D.39048_1 is not a constant and thus > - -Warray-bounds does not issues a warning. > > > The patch I've got queued up will isolate the path 8->9 (to optimize > elsewhere). This results in a new block which looks like: > > temp = PHI (4294967295); > D.39016_xx = default_target_hard_regs.x_fixed_regs[temp]; > > We then propagate the constant into the use of temp triggering the > - -Warray-bounds warning. > > This is caused by this code fragment: > >> /* Any constant, or pseudo with constant equivalences, may >> require reloading from memory using the pic register. */ >> if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM >> && fixed_regs[PIC_OFFSET_TABLE_REGNUM]) >> bitmap_set_bit (regular_block_artificial_uses, >> PIC_OFFSET_TABLE_REGNUM); > > combined with this code from the x86 backend: > >> #define PIC_OFFSET_TABLE_REGNUM \ >> ((TARGET_64BIT && ix86_cmodel == CM_SMALL_PIC) \ >> || !flag_pic ? INVALID_REGNUM \ >> : reload_completed ? REGNO (pic_offset_table_rtx) \ >> : REAL_PIC_OFFSET_TABLE_REGNUM) > > > While the new code can significantly improve path isolation, it's unable > to fully isolate the paths in this code, leading to the partial > isolation and exposing the constant propagation in the dead path which > triggers -Warray-bounds warning. > > I'm hoping the ideas I'm working on for revamping how we handle path > isolation may fix this, but it's hard to be sure right now. In the mean > time, this patch fixes the instances where the next improvements to jump > threading expose the bogus -Warray-bounds warning. > > Bootstrapped and regression tested on x86_64-unknown-linux-gnu. OK for > mainline?
Ok. Thanks, Richard. > Thanks, > Jeff > > > > > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.11 (GNU/Linux) > Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ > > iQEcBAEBAgAGBQJN94aoAAoJEBRtltQi2kC7DwkIAI2zu87P0mqwf+NzI3BAPQpU > GQl9d2Lw4z7diUfn7k+q2OqZMaoof9L0CqvhqC07Pz+UGzpke28o2WoS2Jrwxbj9 > eQzC/H5DcAXmazvkwpe0BphvtqD+2Puz3pilQG1Nyopi1xJB5aKhC55VLntQuAvy > +yaw/ozJ/d0Gt9myR/NXLe0NPfRycDeuC6U+iYRolJ7I/PxP/gZZ5dW68xakstLp > oaQOakKmTres7CMWqG6ZV+5KJyQU92rnkp4ympKZGkciK1yI7Bl8fA87SqY/QkzN > eDoGP37hQnJZkh39QLQjOZCfU5ywVAP81BnYsjaeSAOEd/SdQA63nIzVhGXoDEA= > =K4dB > -----END PGP SIGNATURE----- >
