<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Jun 26, 2013, at 9:37 AM, Tim Northover <<a href="mailto:t.p.northover@gmail.com">t.p.northover@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Hi Mihail,<br><br>+ template<unsigned width, unsigned scale, bool positive><br>+ bool isMemoryOffset() const {<br><br>This seems rather general given that it's just used once. I'd suggest<br>either stripping it back or doing a separate refactoring to<br>consolidate things.<br><br>Also, unless I'm mistaken its constraints are appropriate for 2s<br>complement arithmetic while ARM usually uses sign/magnitude for its<br>offsets. At the very least this assumption should be commented.<br><br>+ return MO.getImm() >> 2;<br><br>I think the preferred approach is to use an already-shifted MCOperand<br>so that invalid instructions can't exist by design. That removes the<br>need for the decoder, but makes your AsmParser render methods (which<br>currently seem pretty much identical to addImm) useful again. CodeGen<br>may also need changing if it ever generates a truly immediate ADR<br>(doubtful).<br><br></div></blockquote><div dir="auto"><br></div><div dir="auto">Yes, exactly right.</div><div dir="auto"><br></div><div dir="auto">-Jim</div></div></body></html>