On Mon, 2008-08-18 at 08:50 -0700, Eli Friedman wrote:> On Mon, Aug 18, 2008 at 6:31 AM, <Sachin.Punyani at microchip.com> wrote: > > assert(Res.getValueType() == N->getValueType(0) && N->getNumValues() == 1 && > > "Invalid operand expansion"); > > > > LOAD node has two values but the assertion checks N->getNumValues() == 1 > > which is not letting us change load operation. >Besides the assert, the call to ReplaceValueWith(SDValue(N, 0), Res); also looks incorrect, because the back-end lowered load will return two values as well. - Sanjiv
On Tue, Aug 19, 2008 at 8:07 AM, sanjiv gupta <sanjiv.gupta at microchip.com> wrote:> On Mon, 2008-08-18 at 08:50 -0700, Eli Friedman wrote: >> On Mon, Aug 18, 2008 at 6:31 AM, <Sachin.Punyani at microchip.com> wrote: >> > assert(Res.getValueType() == N->getValueType(0) && N->getNumValues() == 1 && >> > "Invalid operand expansion"); >> > >> > LOAD node has two values but the assertion checks N->getNumValues() == 1 >> > which is not letting us change load operation. >> > Besides the assert, the call to > ReplaceValueWith(SDValue(N, 0), Res); > > also looks incorrect, because the back-end lowered load will return two > values as well.Oh, I see what you mean! Sorry, I missed the issue originally. Definitely looks like a bug. I think the correct solution is that if custom legalization returns a node, ReplaceNodeWith should be used rather than the existing codepath. -Eli
On Tue, Aug 19, 2008 at 8:40 AM, Eli Friedman <eli.friedman at gmail.com> wrote:> On Tue, Aug 19, 2008 at 8:07 AM, sanjiv gupta > <sanjiv.gupta at microchip.com> wrote: >> On Mon, 2008-08-18 at 08:50 -0700, Eli Friedman wrote: >>> On Mon, Aug 18, 2008 at 6:31 AM, <Sachin.Punyani at microchip.com> wrote: >>> > assert(Res.getValueType() == N->getValueType(0) && N->getNumValues() == 1 && >>> > "Invalid operand expansion"); >>> > >>> > LOAD node has two values but the assertion checks N->getNumValues() == 1 >>> > which is not letting us change load operation. >>> >> Besides the assert, the call to >> ReplaceValueWith(SDValue(N, 0), Res); >> >> also looks incorrect, because the back-end lowered load will return two >> values as well. > > Oh, I see what you mean! Sorry, I missed the issue originally. > Definitely looks like a bug. I think the correct solution is that if > custom legalization returns a node, ReplaceNodeWith should be used > rather than the existing codepath.Actually, it should probably be using TLI.ReplaceNodeResults as well, like the way PromoteIntegerResult calls into the target for custom legalization. -Eli