<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
<HTML>
<HEAD>
<META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=iso-8859-1">
<META NAME="Generator" CONTENT="MS Exchange Server version 6.5.7653.38">
<TITLE>RE: [LLVMdev] PIC16 backend for llvm 2.5</TITLE>
</HEAD>
<BODY>
<!-- Converted from text/plain format -->
<P><FONT SIZE=2>Duncan,<BR>
We have code freeze approaching. Should I check-in this revised patch?<BR>
I ran make check and it passes.<BR>
<BR>
- Sanjiv<BR>
<BR>
<BR>
-----Original Message-----<BR>
From: llvmdev-bounces@cs.uiuc.edu on behalf of sanjiv gupta<BR>
Sent: Mon 1/19/2009 11:51 PM<BR>
To: Duncan Sands<BR>
Cc: llvmdev@cs.uiuc.edu<BR>
Subject: Re: [LLVMdev] PIC16 backend for llvm 2.5<BR>
<BR>
On Sun, 2009-01-18 at 20:28 +0100, Duncan Sands wrote:<BR>
> Hi Sanjiv,<BR>
><BR>
> > + /// CustomLowerOperation - This callback is invoked for operations that are<BR>
> > + /// unsupported by the target, which are registered to use 'custom' lowering,<BR>
> > + /// and whose defined values are all legal.<BR>
><BR>
> and whose defined values are all legal -> and whose return values all have legal types<BR>
><BR>
Taken care of.<BR>
<BR>
> > + /// If the target has no operations that require custom lowering, it need not<BR>
> > + /// implement this.<BR>
><BR>
> This comment and the name seem too generic for me. This method is for use when<BR>
> an operand has an illegal type, right? Yet the comment makes no mention of this.<BR>
> There's also of no mention of the fact that you are allowed to not place anything<BR>
> in Results, and what that means.<BR>
><BR>
Taken care of.<BR>
<BR>
> Also, is there any reason not to use ReplaceNodeResults rather than introducing<BR>
> a new method for type legalization?<BR>
><BR>
ReplaceNodeResults and LowerOperation have been used for different<BR>
purposes by different targets, and have been implemented differently.<BR>
The former is meant to legalize illegal value types and the latter is<BR>
meant to legalize operations with illegal operands. So they might need<BR>
different treatments.<BR>
<BR>
> > + case ISD::ANY_EXTEND: <BR>
> > + Results.push_back(PromoteIntOp_ANY_EXTEND(N)); break;<BR>
><BR>
> This is wrong if PromoteIntOp_ANY_EXTEND returned a value with<BR>
> no node. Likewise for all the others. Better I think to simply<BR>
> handle the custom case immediately and return rather than trying<BR>
> to share code with these other cases.<BR>
><BR>
Taken care of.<BR>
<BR>
> Also, you could just make DAGTypeLegalizer::CustomLowerResults<BR>
> more general, and use that.<BR>
><BR>
Taken care of.<BR>
<BR>
The revised patch is attached.<BR>
<BR>
> Ciao,<BR>
><BR>
> Duncan.<BR>
<BR>
</FONT>
</P>
</BODY>
</HTML>