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

[Jay Foad via Phabricator via Openmp-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPU.td:471
+  "true",
+  "Encoding format GFX10_A"
+>;
----------------
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?


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:7341
+    if (!Subtarget->hasGFX10_AEncoding())
+      emitRemovedIntrinsicError(DAG, DL, Op.getValueType());
+
----------------
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.


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

https://reviews.llvm.org/D103663