[PATCH] Fix implementation for Thumb ADR instruction

Tim Northover t.p.northover at gmail.com
Wed Jun 26 09:37:27 PDT 2013


Hi Mihail,

+  template<unsigned width, unsigned scale, bool positive>
+  bool isMemoryOffset() const {

This seems rather general given that it's just used once. I'd suggest
either stripping it back or doing a separate refactoring to
consolidate things.

Also, unless I'm mistaken its constraints are appropriate for 2s
complement arithmetic while ARM usually uses sign/magnitude for its
offsets. At the very least this assumption should be commented.

+  return MO.getImm() >> 2;

I think the preferred approach is to use an already-shifted MCOperand
so that invalid instructions can't exist by design. That removes the
need for the decoder, but makes your AsmParser render methods (which
currently seem pretty much identical to addImm) useful again. CodeGen
may also need changing if it ever generates a truly immediate ADR
(doubtful).

Tim.



More information about the llvm-commits mailing list