Type Legalizer - Load handling problem

Hi All,

I have some doubt in LLVM Type Legalizer.

How will LOAD:i8 with an i16 operand be lowered in type legalizer? (i16 type is not legal for our target)

Following assertion in function ExpandIntegerOperand (file LegalizeIntegerTypes.cpp) is not allowing us to change LOAD node.

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.

Also in the first check of the insertion, 0th value type (MVT::Other (chain) for load) of the node N is being compared with the value type of Res. It is not trying to compare 1st value (which is i8) of LOAD with the 1st value of Res.

Regards

Sachin

I don't really know the right answer here, but custom-legalizing to a
backend-specific load operation should be feasible, although it's a
bit messy.

-Eli

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

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

Actually, it should probably be using TLI.ReplaceNodeResults as well,
like the way PromoteIntegerResult calls into the target for custom
legalization.

-Eli

Actually, it should probably be using TLI.ReplaceNodeResults as well,
like the way PromoteIntegerResult calls into the target for custom
legalization.

Definitely makes much more sense to me.

Hi,

How will LOAD:i8 with an i16 operand be lowered in type legalizer? (i16
type is not legal for our target)

what does "LOAD:i8 with an i16 operand" mean? Are you saying that the
type of a pointer is i16, and you are loading an i8 value from the
pointed to location? If so, you are in trouble because many parts of
the code generator assume that the type of a pointer is legal.

Following assertion in function ExpandIntegerOperand (file
LegalizeIntegerTypes.cpp) is not allowing us to change LOAD node.

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.

Yup, in this case you need to return an SDValue with null Val field,
and take care of replacing values yourself.

Also in the first check of the insertion, 0th value type (MVT::Other
(chain) for load) of the node N is being compared with the value type of
Res. It is not trying to compare 1st value (which is i8) of LOAD with
the 1st value of Res.

Since the assertion only allows the node to have one value, this is not
a problem.

Ciao,

Duncan.

> LOAD node has two values but the assertion checks N->getNumValues() == 1
> which is not letting us change load operation.

Yup, in this case you need to return an SDValue with null Val field,
and take care of replacing values yourself.

That said, it looks like it is done this way because no-one needed
anything more. It could easily be changed to handle the case of any
number of return values.

Ciao,

Duncan.

Yes we have 16-bit pointers but only 8-bit operations and 8-bit
registers.

For indirect loads/stores, the 16-bit pointer gets loaded into a
register pair.

- Sanjiv

Why not use ReplaceNodeWith ?

- Sanjiv

> That said, it looks like it is done this way because no-one needed
> anything more. It could easily be changed to handle the case of any
> number of return values.
>
Why not use ReplaceNodeWith ?

I just took a look and it isn't that simple. This code is used
for example to replace
  i32 = truncate 0x2374b48
with the first result of
  i32,ch = load 0x2356390, 0x2357188, 0x2356b88 <0x236ee80:0> alignment=4
This works fine right now. It fails with ReplaceNodeWith because the
number of results differs. It could be made to work by using a MergeValues
node to produce a node with one result out of the load, and similarly for
the other examples of this kind of thing.

Ciao,

Duncan.

I think we will have to go that way since I may want to replace a "store" node also
(A store node does not produce any results). In this case ReplaceNodeWith () is my only way out.

Thanks,
Sanjiv

I think we will have to go that way since I may want to replace a "store" node also
(A store node does not produce any results). In this case ReplaceNodeWith () is my only way out.

I agree we should go that way because it is more logical. However
you can also work around the problem by calling ReplaceNodeWith
yourself, and returning an SDValue with null node.

Ciao,

Duncan.

> I think we will have to go that way since I may want to replace a "store" node also
> (A store node does not produce any results). In this case ReplaceNodeWith () is my only way out.

I agree we should go that way because it is more logical. However
you can also work around the problem by calling ReplaceNodeWith
yourself, and returning an SDValue with null node.

How do we access ReplaceNodeWith in TLI ?

- Sanjiv

How do we access ReplaceNodeWith in TLI ?

I hadn't understood you wanted to custom lower:
I assumed you wanted to teach legalize types
what to do if the pointer type is illegal, in
the generic code. Indeed if you want to custom
lower then there isn't much choice.

Ciao,

Duncan.