<div dir="ltr">More on that front. I commented out the part of DAGTypeLegalizer::ExpandIntRes_ADDSUB that legalize using ADDC/ADDE so it falls back on using UADDO. The generated DAG on X86 looks legit, but I end up with 66 test failures, most of them seems to be due to trunc i8 X to i32 ending up being generated. Help !<br></div><div class="gmail_extra"><br><div class="gmail_quote">2017-02-09 11:21 GMT+01:00 Amaury SECHET <span dir="ltr"><<a href="mailto:deadalnix@gmail.com" target="_blank">deadalnix@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div>So the plan would be:<br><br></div>1/ Use UADDO instead of ADDC.<br>2/ Deprecate and remove ADDC.<br></div>3/ Have ADDE/SUBE accept a boolean as carry input instead of a glue.<br><br></div>There are still one thing I don't really understand: how come the X86 backend end up avoiding turnning into the problem altogether to begin with. I think that's an important piece of the puzzle.<br><div><div><br></div></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">2017-02-09 5:48 GMT+01:00 James Y Knight <span dir="ltr"><<a href="mailto:jyknight@google.com" target="_blank">jyknight@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I'd think i1 would be the proper and correct choice for a carry flag for the generic instruction. I expect that would also make UADDO/USUBO redundant with ADDC/SUBC (which would seem a good outcome).<div><br></div><div>You'd need to make sure the right thing happened when converting from ADDC's 1-bit carry in/out to X86ISD::AD[DC]'s EFLAGS i/o. Right now the conversion can get away with assuming that the only thing which can be used as input to ADDC is the glue-output of ADD/ADDC. That would no longer be the case once they're using a normal value.</div><div><br></div><div>(Actually, I wonder if it would be possible to represent x86's EFLAGS.CARRY as a 1-bit subreg of EFLAGS, and use that on input to X86ISD::ADC; that may allow cheaper spills/reloads if it only has to save the one bit?).</div></div><div class="m_-4382279149067515408HOEnZb"><div class="m_-4382279149067515408h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 8, 2017 at 7:28 PM, Amaury SECHET <span dir="ltr"><<a href="mailto:deadalnix@gmail.com" target="_blank">deadalnix@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Well I don't think this would break most backend. The opcode is generate only if the backend allows so to boot, so many backend actually do not use it at all.<br><br></div>Anyway, changing these opcode is kind of much broader scope than what I anticipated, but I maybe can make it work. What do you think would be an appropriate type for the carry instead of glue ?<br></div><div class="m_-4382279149067515408m_4714188105971680614HOEnZb"><div class="m_-4382279149067515408m_4714188105971680614h5"><div class="gmail_extra"><br><div class="gmail_quote">2017-02-08 21:19 GMT+01:00 James Y Knight <span dir="ltr"><<a href="mailto:jyknight@google.com" target="_blank">jyknight@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>I don't think that'd work, because it leaves all other backends broken. AFAICT, your transform is simply not a legal transform, with the way the ADDC/ADDE opcodes are currently defined, and to do it you really need to fix the opcode definitions to not involve glue, first.</div><div><br></div><div>I also note that your transform doesn't actually trigger at all on this particular test case on x86, because the dag ends up looking like (uaddo X, (adde Y, 0, Carry)), which the transform doesn't match. That is not really relevant to the issue here, because the x86 backend does work even if the transform gets triggered (replacing the final store with "store i64 %add37, i64* %r, align 8" will make this happen)</div><div><br></div></div><div class="m_-4382279149067515408m_4714188105971680614m_-1541643088576754441HOEnZb"><div class="m_-4382279149067515408m_4714188105971680614m_-1541643088576754441h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 7, 2017 at 5:44 PM, Amaury SECHET <span dir="ltr"><<a href="mailto:deadalnix@gmail.com" target="_blank">deadalnix@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Making this a value instead of a glue looks like a good longer term solution, but it doesn't quite cut it as a short term one. It would require to change a fair amount of code in the DAG stack, plus in each backend.<br><br></div>@James, <br>do you think doing something similar to what the X86 backend does in the PowerPC one would cut it ?<br><br></div><div class="m_-4382279149067515408m_4714188105971680614m_-1541643088576754441m_-6552768533361388853HOEnZb"><div class="m_-4382279149067515408m_4714188105971680614m_-1541643088576754441m_-6552768533361388853h5"><div class="gmail_extra"><br><div class="gmail_quote">2017-02-07 22:51 GMT+01:00 Nemanja Ivanovic <span dir="ltr"><<a href="mailto:nemanja.i.ibm@gmail.com" target="_blank">nemanja.i.ibm@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Would it not make sense to refactor the code so those don't use glue rather than emitting them with glue and then getting rid of it. There are times when we would like to emit these in separate blocks but can't (presumably because of the glue).<br></div><div class="m_-4382279149067515408m_4714188105971680614m_-1541643088576754441m_-6552768533361388853m_-4401159160715364870HOEnZb"><div class="m_-4382279149067515408m_4714188105971680614m_-1541643088576754441m_-6552768533361388853m_-4401159160715364870h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 7, 2017 at 9:15 PM, James Y Knight via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">That's seems really odd that ADDC/ADDE uses glue there, instead of a plain value.<div><br></div><div>The x86 backend has code that converts the glue into a value, which is why it wasn't affected.... (LowerADDC_ADDE_SUBC_SUBE).<br></div></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_-4382279149067515408m_4714188105971680614m_-1541643088576754441m_-6552768533361388853m_-4401159160715364870m_-8249861136748625907h5">On Tue, Feb 7, 2017 at 2:48 PM, Amaury SECHET via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="m_-4382279149067515408m_4714188105971680614m_-1541643088576754441m_-6552768533361388853m_-4401159160715364870m_-8249861136748625907h5"><div dir="ltr"><div><div><div><div><div><div>Long story short: <a href="https://llvm.org/bugs/show_bug.cgi?id=31890" target="_blank">https://llvm.org/bugs/show_bug<wbr>.cgi?id=31890</a><br><br></div>The backend fails to schedule a given DAG, the reason being that there is an instruction and it glue that needs to be broken apart as they can't be scheduled consecutively. See attached file for a picture of the DAG.<br><br></div>Not sure what's the best course of action is, and not sure why this isn't a problem for the X86 backend either. I'm looking for advice on the best course of actions. As I see it, the option are:<br></div><br>1/ add extra logic in the DAGCombiner to make sure this doesn't happen. I don't see a way this could be done cheaply and overall I don't think this is the best option/<br></div>2/ Have the ScheduleDAG machinery detect this case and break up the glue, for instance via breaking up (adde X, Y, Carry) into (add (add X, Y)n (adde 0, 0, Carry)) or something alike when the situation present itself.<br></div>3/ Do whatever the X86 backend does, which I'm not sure what it is.<br><br></div>Advice ?<br><div><div><div><br></div><div>Thanks in advance,<br><br></div><div>Amaury SECHET<br></div><div><br></div></div></div></div>
<br></div></div>______________________________<wbr>_________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
<br></blockquote></div><br></div>
<br>______________________________<wbr>_________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>