[PATCH] D78698: [VE] Implements minimum MC layer for VE

Simon Moll via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 29 05:19:57 PDT 2020


simoll added a comment.

In D78698#2007297 <https://reviews.llvm.org/D78698#2007297>, @kaz7 wrote:

> Thank you for the comments.  I'm also considering the size of this patch.   I'll split it as you suggested after a week and half holidays.  How should I leave this patch?  Close this and open divided patches newly?  Let me know the recommended way.  Thanks in advance.


It's up to you really. I'd keep this patch open and add its parts as related revisions. Once all the child patches are committed, this diff can be closed/abandoned.



================
Comment at: llvm/lib/Target/VE/VEInstrInfo.td:261
 def brtarget32 : Operand<OtherVT> {
-  let EncoderMethod = "getBranchTarget32OpValue";
 }
----------------
kaz7 wrote:
> simoll wrote:
> > Is this a related change? (same in l265).
> Kind of.  EncoderMethod will be called from MC layer.  However, we merged not-defined encoder method like this, so I needed to remove it once until we add actual encoder function in following patches.  I will divide patches like below.
> 
>   # Fixes for the instruction bit encoding.
>   # The 'EM_VE' ELF changes
>   # Fixes for undefined methods like this.
>   # The VEAsmParser and tests.
Sounds good.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78698/new/

https://reviews.llvm.org/D78698





More information about the llvm-commits mailing list