arsenm added a comment.

In D77910#1989163 <https://reviews.llvm.org/D77910#1989163>, @b-sumner wrote:

> In D77910#1988807 <https://reviews.llvm.org/D77910#1988807>, @arsenm wrote:
>
> > In D77910#1981828 <https://reviews.llvm.org/D77910#1981828>, @b-sumner 
> > wrote:
> >
> > > In D77910#1981429 <https://reviews.llvm.org/D77910#1981429>, @arsenm 
> > > wrote:
> > >
> > > > In D77910#1976171 <https://reviews.llvm.org/D77910#1976171>, @b-sumner 
> > > > wrote:
> > > >
> > > > > I don't think we can guarantee this is or will be supported on all 
> > > > > devices.  The language runtime makes this decision.
> > > >
> > > >
> > > > We don't need to worry about theoretical devices. We should know the 
> > > > properties of the driver from -amdhsa, -amdpal, -mesa3d
> > >
> > >
> > > It takes more than support in the ISA for some features.  The OpenCL 
> > > driver may not want to support a given optional feature, e.g. images.  
> > > I'm not opposed to defaults, but if the driver chooses to not support 
> > > images, it needs to be able to prevent `__IMAGE_SUPPORT__` from being 
> > > defined.  Conformance will fail if the runtime and compiler are not 
> > > consistent.
> >
> >
> > The driver details should be captured by the the triple. If some weird 
> > driver decided to do something different, we would need to add a new triple 
> > for it. We don't have such a driver, so I don't see why worry about it. 
> > It's possible to work around with undef and redef in an implicitly included 
> > header. We need to fix properties of the driver based on the target to have 
> > perfectly matching offline compilation
>
>
> I don't see anywhere in the triple talking about driver specific details, 
> unless you would use the environment?  That seems like overkill to me.  But 
> again, I'm not opposed to defaults, and as long as the driver can override 
> them, this should be OK.


The OS is the driver. It doesn't need to specifically encode these details; the 
OS should encode properties of the driver environment. Anything using -amdhsa 
should be reporting image support


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77910/new/

https://reviews.llvm.org/D77910



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D77910: AMDGPU: Def... Matt Arsenault via Phabricator via cfe-commits

Reply via email to