[PATCH] D10537: [mips] Add support for branch-likely pseudo-instructions

Srdjan Obucina via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 7 06:28:18 PDT 2015


obucina added a comment.

> Hi,

> 

> I believe you already have a conditional LGTM for this patch (for diff 28228) and that the quoted text above contains the only remaining issue. It was asking that you remove the AsmToken::Identifier code (because it introduces another way to parse labels which is already handled elsewhere) and fix the wrong-error issue it was intended to fix in a later patch that deals with other cases too.


OK, I will do this, but now there is another problem to be solved. As You noticed...

> > PredicateControl moved from CondBranchPseudo to the MipsAsmPseudoInst

> 

> 

> I'm not sure why this change was made but it seems to have been reverted in the next diff. I'm not sure if that was intended so I thought I should mention it.


There is a problem with ULHU/ULW tests, similar to the one with parsing labels. When PredicateControl is attached to the MipsAsmPseudoInst, second parameter of ULHU/ULW cant be parsed correctly, causing tests to fail. Moving it to CondBranchPseudo is workaround to make the patch applicable.

Please take a look at https://dmz-portal.mips.com/bugz/show_bug.cgi?id=2190

I agree this is not the smartest way to fix it and we have internal communication about possible ways to resolve this.

At the time of creating these diffs, ULHU/ULW support was not implemented, it was added few days after first patch here. This is why this error emerged in later versions. The same problem prevents another patch to be committed, the one for ROL/DROL support, since it also requires PredicateControl attached to MipsAsmPseudoInst.


http://reviews.llvm.org/D10537





More information about the llvm-commits mailing list