[Openmp-commits] [PATCH] D103663: [AMDGPU] Add gfx1013 target

Brendon Cahoon via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Tue Jun 8 10:21:06 PDT 2021


bcahoon marked an inline comment as done.
bcahoon added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPU.td:471
+  "true",
+  "Encoding format GFX10_A"
+>;
----------------
foad wrote:
> I realise you're just following the precedent set by GFX10_B, but is this terminology actually used in any documentation anywhere? And if not could we describe it a little better here?
I changed the description to be specific w.r.t what the target feature enables.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:7341
+    if (!Subtarget->hasGFX10_AEncoding())
+      emitRemovedIntrinsicError(DAG, DL, Op.getValueType());
+
----------------
foad wrote:
> bcahoon wrote:
> > rampitec wrote:
> > > return emitRemovedIntrinsicError();
> > I've changed this to return. Thanks for catching that. But, it returns a UNDEF value instead of SDValue() so that it doesn't crash. I can change the behavior if that's preferred. 
> Personally I would follow all the existing precedents and "return emitRemovedIntrinsicError(...)". I don't see any value in deliberately trying to make the compiler crash harder.
The handling of diagnostic errors is a little inconsistent in ISelLowering. Sometimes SDValue() is returned and other times it's Undef. I have it return SDValue() so that the failure mode is consistent with how GlobaISel handles these intrinsics. It's probably worth a discussion to decide how best to handle diagnostic errors. I'm happy to submit a follow-on patch as needed.

As an aside, return emitRemovedIntrinsicError() isn't enough because the intrinsic has both a return value and a chain edge. So, something like return DAG.getMergeValues({emitRemovedIntrinsicError(), Op.getValue(0)}, DL) is needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103663



More information about the Openmp-commits mailing list