[llvm-commits] Intrinsic address space patch

[Dan Gohman]

Hi Mon Ping,

Looks good; I like the iPTRAny idea. Comments below.

 >        // Print function.
 >        std::string OpVTStr;
 > -      if (OpVT == MVT::iPTR) {
 > +      if (OpVT == MVT::iPTR || OpVT == MVT::iPTRAny) {
 >          OpVTStr = "_iPTR";

It probably doesn't break anything, but it seems a little confusing
to have iPTRAny rendered as iPTR here. Could this print iPTRAny
separately?

 > @@ -495,7 +496,8 @@
 >      setTypes(ExtVTs);
 >      return true;
 >    }
 > -  if (getExtTypeNum(0) == EMVT::isInt && ExtVTs[0] == MVT::iPTR) {
 > +  if (getExtTypeNum(0) == EMVT::isInt &&
 > +      (ExtVTs[0] == MVT::iPTR || ExtVTs[0] == MVT::iPTR)) {

Looks like the second iPTR here should be iPTRAny.

 > @@ -527,6 +529,7 @@
 >    case EMVT::isFP : OS << ":isFP"; break;
 >    case EMVT::isUnknown: ; /*OS << ":?";*/ break;
 >    case MVT::iPTR:  OS << ":iPTR"; break;
 > +  case MVT::iPTRAny:  OS << ":iPTR"; break;

As above, can iPTRAny be rendered as "iPTRAny"?

 > +      }
 > +    } else if (VT == MVT::iPTRAny) {
 > +      // Outside of TableGen, we don't distinguish iPTRAny (to any  
address
 > +      // space) and iPTR. In the verifier, we can not distinguish  
which case
 > +      // we have so allow either case to be legal.
 > +      if (const PointerType* PTyp = dyn_cast<PointerType>(Ty)) {
 > +        Suffix += ".p" + utostr(PTyp->getAddressSpace()) +
 > +        MVT::getMVT(PTyp->getElementType()).getMVTString();
 > +      }
 > +      else {

Put the else on the same line with the } please :-).

Thanks,

Dan

On Jul 25, 2008, at 3:51 PM, Mon P Wang wrote:

> Hi,
>
> Here is an updated patch for intrinsic address space.  The effects  
> are the same.  The main difference between this patch and the other  
> one is that instead of assuming a pointer may or may not be  
> overloaded, I have introduced a new type iPTRAny which indicates  
> that the  pointer can be overloaded based on an address spaces.   
> This would allow us to overload intrinsics based only on a pointer.   
> The iPTRAny should only be used in TableGen (and part of the  
> Verifier)  and has the same semantics of iPTR except for the  
> overloading.  Please let me know if you have any comments.
>
>   -- Mon Ping
>
>
> <addrspace4.patch>
>
>
>
> On Thu, July 17, 2008 3:56 pm, Mon P Wang wrote:
>>
>> Here is a patch to enable intrinsic to have pointers to different
>> address spaces.  This changes overloaded intrinsics with pointer
>> arguments to pass also an address space as part of their name.
>>    For example,  atomic.load.add.i32 => atomic.load.add.i32.p0i32
>>
>> The above syntax indicates that the result is i32 with a pointer
>> argument to address space 0 (generic address space) whose domain type
>> is i32.  The patch here doesn't include the documentation change but
>> that will be included as part of the checkin.  Let me know if you  
>> have
>> any comments.
>>
>> -- Mon Ping
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits