http://llvm.org/bugs/show_bug.cgi?id=7992
Summary: illegal operations introduced by
DAGCombiner::ReduceLoadWidth
Product: new-bugs
Version: trunk
Platform: Macintosh
OS/Version: MacOS X
Status: NEW
Severity: normal
Priority: P
Component: new bugs
AssignedTo: [email protected]
ReportedBy: [email protected]
CC: [email protected]
Below is the email exchange describing the problem. This is present in the SVN
head as of 2010-08-25.
In looking at isLoadExtLegal in TargetLowering.h there are number of other
similar functions (e.g. isIndexedLoadLegal, isTruncStoreLegal,
isIndexedStoreLegal) that could potentially have similar issues.
On Mon, Jul 19, 2010 at 5:21 PM, JP Bonn <[email protected]> wrote:
> > 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?
Post-legalization, operations with custom lowerings shouldn't be
generated by the DAGCombiner; isLoadExtLegal should probably take a
"IsAfterLegalization" bool as an argument or something like that.
-Eli
--
Configure bugmail: http://llvm.org/bugs/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
LLVMbugs mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/llvmbugs
