<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Nov 14, 2017, at 4:43 PM, Volkan Keles <<a href="mailto:vkeles@apple.com" class="">vkeles@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br class="Apple-interchange-newline"><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite" class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;"><div class="">On Nov 13, 2017, at 4:32 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com" class="">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite" class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Nov 13, 2017, at 8:51 AM, Kristof Beyls via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="">reviews@reviews.llvm.org</a>> wrote:<br class=""><br class="">kristof.beyls added inline comments.<br class=""><br class=""><br class="">================<br class="">Comment at: lib/CodeGen/GlobalISel/LegalizerInfo.cpp:194-201<br class="">+ unsigned Op = MI.getOperand(i).getReg();<br class="">+ // G_UNMERGE_VALUES has variable number of operands, but there is only<br class="">+ // one source type and one destination type as all destinations must be the<br class="">+ // same type. So, get the last operand if TypeIdx == 1.<br class="">+ if (MI.getOpcode() == TargetOpcode::G_UNMERGE_VALUES && TypeIdx == 1) {<br class="">+ Op = MI.getOperand(MI.getNumOperands() - 1).getReg();<br class="">+ }<br class="">----------------<br class="">To make this easier to read, I think it may make sense to factor this out into a separate function, e.g. with a name like "getTyFromTypeIdx(MI, TypeIdx)". I'm not sure on the name, but doing it this way would make this code a lot easier to understand, I think.<br class=""><br class=""><br class="">================<br class="">Comment at: lib/Target/AArch64/AArch64LegalizerInfo.cpp:359<br class="">+ for (const auto &Ty :<br class="">+ {s32, s64, s96, s128, s192, v2s32, v3s32, v4s32, v2s64, v4s64, v6s64}) {<br class="">+ setAction({G_MERGE_VALUES, Ty}, Legal);<br class="">----------------<br class="">I haven't thought this through at all, but I'm a bit surprised v8s64 also isn't legal if v6s64 is?<br class="">Similarly for S256 if s192 is legal?<br class=""></blockquote><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;">FWIW, I believe we should just specify the size (instead of full types) for operations that deal only with bit manipulation (merge_value, phi, bitwise operation, ld/st).</span><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"></div></blockquote><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="" style="margin: 0px; font-stretch: normal; line-height: normal; font-family: "Helvetica Neue"; color: rgb(69, 69, 69);">I think it’s good to legalize these operations based on full types because legalization actions might be different, especially for vector types.</div></div></div></blockquote><div><br class=""></div><div>Given that those instructions only deal with bag of bits, I don’t see why the full types are relevant.</div><div>What am I missing?</div><br class=""><blockquote type="cite" class=""><div class=""><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="" style="margin: 0px; font-stretch: normal; line-height: normal; font-family: "Helvetica Neue"; color: rgb(69, 69, 69);"> Otherwise, we’d need to mark it as Custom, check the full type and replace it with something else even if it’s legal.</div><div class="" style="margin: 0px; font-stretch: normal; line-height: normal; font-family: "Helvetica Neue"; color: rgb(69, 69, 69); min-height: 14px;"><br class=""></div><div class="" style="margin: 0px; font-stretch: normal; line-height: normal; font-family: "Helvetica Neue"; color: rgb(69, 69, 69);">Volkan</div></div><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite" class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;"><div class=""><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite" class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""><br class=""><a href="https://reviews.llvm.org/D39823" class="">https://reviews.llvm.org/D39823</a></blockquote></div></blockquote></div></blockquote></div><br class=""></body></html>