DAGCombiner::ReduceLoadWidth() does the following: /// ReduceLoadWidth - If the result of a wider load is shifted to right of N /// bits and then truncated to a narrower type and where N is a multiple /// of number of bits of the narrower type, transform it to a narrower load /// from address + N / num of bits of new type. If the result is to be /// extended, also fold the extension to form a extending load. The problem I'm running into is our loads are custom lowered. Our architecture does not support loads smaller than 32 bits so we change these loads into 32 bit loads. Unfortunately ReduceLoadWidth() then lowers them back into 16 bit loads. Is this a bug in ReduceLoadWidth() or is there something I'm missing? Alternatively, I guess I could create target specific nodes for the lowered loads. thanks, JP
Hi JP,> DAGCombiner::ReduceLoadWidth() does the following: > /// ReduceLoadWidth - If the result of a wider load is shifted to right of N > /// bits and then truncated to a narrower type and where N is a multiple > /// of number of bits of the narrower type, transform it to a narrower load > /// from address + N / num of bits of new type. If the result is to be > /// extended, also fold the extension to form a extending load. > > The problem I'm running into is our loads are custom lowered. Our > architecture does not support loads smaller than 32 bits so we change > these loads into 32 bit loads. Unfortunately ReduceLoadWidth() then > lowers them back into 16 bit loads. > > Is this a bug in ReduceLoadWidth() or is there something I'm missing?I think it's a bug. When running after type legalization, the DAG combiner should not introduce any illegal types. When running after operation legalization, the DAG combiner should not introduce any illegal operations. Consider this code from ReduceLoadWidth: if (LegalOperations && !TLI.isLoadExtLegal(ISD::SEXTLOAD, ExtVT)) return SDValue(); Here you see that it checks whether it is only allowed to produce legal operations, and bails out if it would create an illegal extending load. However the later logic in ReduceLoadWidth doesn't do any such checking as far as I can see, which is wrong. Ciao, Duncan.> > Alternatively, I guess I could create target specific nodes for the > lowered loads.
On 7/19/10 10:36 AM, Duncan Sands wrote:> Hi JP, > > >> DAGCombiner::ReduceLoadWidth() does the following: >> /// ReduceLoadWidth - If the result of a wider load is shifted to right of N >> /// bits and then truncated to a narrower type and where N is a multiple >> /// of number of bits of the narrower type, transform it to a narrower load >> /// from address + N / num of bits of new type. If the result is to be >> /// extended, also fold the extension to form a extending load. >> >> The problem I'm running into is our loads are custom lowered. Our >> architecture does not support loads smaller than 32 bits so we change >> these loads into 32 bit loads. Unfortunately ReduceLoadWidth() then >> lowers them back into 16 bit loads. >> >> Is this a bug in ReduceLoadWidth() or is there something I'm missing? >> > I think it's a bug. When running after type legalization, the DAG combiner > should not introduce any illegal types. When running after operation > legalization, the DAG combiner should not introduce any illegal operations. > Consider this code from ReduceLoadWidth: > > if (LegalOperations&& !TLI.isLoadExtLegal(ISD::SEXTLOAD, ExtVT)) > return SDValue(); > > Here you see that it checks whether it is only allowed to produce legal > operations, and bails out if it would create an illegal extending load. > However the later logic in ReduceLoadWidth doesn't do any such checking > as far as I can see, which is wrong. > >It does appear the same test should be applied to the ZEXTLOAD case too. For my case where I do custom lowerings TLI.isLoadExtLegal() considers custom lowerings as being legal Should these tests exclude custom lowerings? TLI.isLoadExtLegal() is not declared as being virtual - should it be? If I override TLI.isLoadExtLegal() and exclude custom lowerings it fixes my problem but is that really the correct solution? Should I just use a target specific node for my custom lowered loads.
Maybe Matching Threads
- [LLVMdev] DAGCombiner::ReduceLoadWidth bug?
- [LLVMdev] DAGCombiner::ReduceLoadWidth bug?
- [LLVMdev] ReduceLoadWidth, DAGCombiner and non 8bit loads/extloads question.
- [LLVMdev] ReduceLoadWidth, DAGCombiner and non 8bit loads/extloads question.
- [LLVMdev] Avoiding load narrowing in DAGCombiner