[PATCH] D17925: [ARM] Add Cortex-R8 to target parser and tablegen

scott douglass via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 10 02:35:21 PST 2016


scott-0 added inline comments.

================
Comment at: lib/Target/ARM/ARM.td:637
@@ +636,3 @@
+                                                         FeatureSlowFPBrcc,
+                                                         FeatureHWDivARM,
+                                                         FeatureHasSlowFPVMLx,
----------------
rengolin wrote:
> scott-0 wrote:
> > t.p.northover wrote:
> > > rengolin wrote:
> > > > R6+ supports HWDIV for both ARM and Thumb.
> > > Good catch. In a possibly related note, what's happening with MCHammer within ARM? 
> > > 
> > > This seems like exactly the sort of issue it should have caught (an encoding that was invalid on LLVM but should have been supported).
> > > 
> > > Abandoning it is obviously fine (it was internal anyway), but I'd sort of been assuming that patches from ARM were backed by that kind of verification (which at least included 2 mostly independent implementations of the same instruction).
> > In this case, ARMv7r has FeatureHWDiv (to enable the Thumb DIV instructions)
> > 
> > [Aside]
> > 
> > > what's happening with MCHammer within ARM?
> > 
> > MC Hammer is still going, but it is based on the features, not the cpus.  It will notice if the feature doesn't correctly enable the instructions, but not if the cpu doesn't enable the feature.
> Yup, looks like copy&paste from the (also wrong) Cortex-R7. We should fix both.
Maybe I'm missing something, but I think both cortex-r7 and cortex-r8 are correct here:  they have FeatureHWDivARM directly and FeatureHWDiv (Thumb) via ARMv7r.


http://reviews.llvm.org/D17925





More information about the llvm-commits mailing list