[PATCH] D101819: [M68k][GloballSel] Adding initial GlobalISel infrastructure

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:3125
 
   if (!DL->isLittleEndian()) {
     // Currently we don't properly handle big endian code.
----------------
RKSimon wrote:
> Would this be better:
> ```
> if (!DL->isLittleEndian() && !CLI->enableBigEndian()) {
> ```
> I'm not sure if we should always emit at least a warning that big-endian gisel isn't properly supported.
+1


================
Comment at: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:3131
+    // TODO : removing reportTranslationError until big endian is supported
+    // reportTranslationError(*MF, *TPC, *ORE, R);
   }
----------------
sushmaunnibhavi wrote:
> aemerson wrote:
> > sushmaunnibhavi wrote:
> > > qcolombet wrote:
> > > > Instead of purely removing it, turn that into a warning.
> > > @qcolombet , @aemerson 
> > > Is it right if I do this instead?
> > > 
> > > ```
> > > if (Triple(MF->getTarget().getTargetTriple()).getArch() != Triple::m68k) {
> > >       reportTranslationError(*MF, *TPC, *ORE, R);
> > >     }
> > > ```
> > > 
> > > This would not call reportTranslationError only for m68k but would not affect any other targets. 
> > It's less preferable than a TLI hook because it encodes target specific behavior into the translator.
> I have created an enableBigEndian() hook which the targets can use if they want to use big-endian. I ran the tests locally and the tests passed. But some of the unit tests fail when I submitted the patch which is not related to globalisel. What could be the reason for that?
> 
> I am new to compilers so wanted to know where I am going wrong.
Which tests are failing? Have you checked if they pass without your changes?


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

https://reviews.llvm.org/D101819