search for: foldphiargopintophi

Hi, AFAICT there are two places where zext instructions may get folded into PHI nodes. One is FoldPHIArgZextsIntoPHI and the other is the more generic FoldPHIArgOpIntoPHI. Now, the former only handles PHIs with more than 2 incoming values, while the latter only handles casts where the source type is legal. This means that for an PHI node with two incoming i8 values, both resulting from `zext i1 * to i8` instructions, both of these functions will refuse to actually...

...29, 2017 at 3:09 PM, Björn Steinbrink via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> Hi, >> >> AFAICT there are two places where zext instructions may get folded into >> PHI nodes. One is FoldPHIArgZextsIntoPHI and the other is the more generic >> FoldPHIArgOpIntoPHI. Now, the former only handles PHIs with more than 2 >> incoming values, while the latter only handles casts where the source type >> is legal. >> >> This means that for an PHI node with two incoming i8 values, both >> resulting from `zext i1 * to i8` instructions, both...

...dea is to ease the restriction in ShouldChangeType > because i1 is special. I tried the patch below, and it works on the > example...and nothing in 'make check' failed. :) > Yeah, that would work for me as well, I just wasn't sure about the implications that has. Simply making FoldPHIArgOpIntoPHI act like FoldPHIArgZextsIntoPHI seemed like the safer option to me, but I wanted feedback on it before creating a PR. Do you want to go ahead with that minimal approach and create a PR yourself? Björn > Index: lib/Transforms/InstCombine/InstructionCombining.cpp > ==========================...

Hi Björn and Daniel, FoldPHIArgOpIntoPHI() is for unary ops (casts), but it calls FoldPHIArgBinOpIntoPHI() for binops which takes care of almost everything else? My minimal patch idea is to ease the restriction in ShouldChangeType because i1 is special. I tried the patch below, and it works on the example...and nothing in 'make check...

...tion in ShouldChangeType >> because i1 is special. I tried the patch below, and it works on the >> example...and nothing in 'make check' failed. :) >> > > Yeah, that would work for me as well, I just wasn't sure about the > implications that has. Simply making FoldPHIArgOpIntoPHI act like > FoldPHIArgZextsIntoPHI seemed like the safer option to me, but I wanted > feedback on it before creating a PR. Do you want to go ahead with that > minimal approach and create a PR yourself? > Your idea probably has the minimal impact while still fixing your problem case...al...

...>>> because i1 is special. I tried the patch below, and it works on the >>> example...and nothing in 'make check' failed. :) >>> >> >> Yeah, that would work for me as well, I just wasn't sure about the >> implications that has. Simply making FoldPHIArgOpIntoPHI act like >> FoldPHIArgZextsIntoPHI seemed like the safer option to me, but I wanted >> feedback on it before creating a PR. Do you want to go ahead with that >> minimal approach and create a PR yourself? >> > > Your idea probably has the minimal impact while still fixi...