qcolombet added inline comments.

================
Comment at: lib/Driver/ToolChains/Clang.cpp:4699
+    else
+      CmdArgs.push_back("-global-isel=0");
+  }
----------------
aemerson wrote:
> qcolombet wrote:
> > qcolombet wrote:
> > > I think that it would be useful to also set -global-isel-abort=2, so that 
> > > users can report problem early.
> > > 
> > > What do you think?
> > > 
> > > 
> > Should we have some kind of "target validation"?
> > What I'd like to avoid is people trying the new allocator on unsupported 
> > target (as in the GISel base classes are not present) that would just crash 
> > the compiler.
> > 
> > Alternatively, we could fix the backend to fallback gracefully/abort 
> > properly in those situation.
> > Right now I believe we would get a segfault on RegisterBankInfo or 
> > something along those lines.
> > I think that it would be useful to also set -global-isel-abort=2, so that 
> > users can report problem early.
> > 
> > What do you think?
> 
> Yes that makes sense, although should we ignore it for ARM64 O0 since we will 
> officially support it?
> 
> > 
> > Should we have some kind of "target validation"?
> > What I'd like to avoid is people trying the new allocator on unsupported 
> > target (as in the GISel base classes are not present) that would just crash 
> > the compiler.
> 
> Perhaps a warning like "GlobalISel support is incomplete for [target]"? I 
> don't know the GISel status for other targets.
> 
> 
> Yes that makes sense, although should we ignore it for ARM64 O0 since we will 
> officially support it?

Sounds sensible. We can advertise the remark option if people want to know what 
is going on.


Repository:
  rC Clang

https://reviews.llvm.org/D42276



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to