[PATCH] D95580: [mlir][spirv] Add support for OpImageType

Weiwei via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 28 10:10:51 PST 2021


Weiwei-2021 added inline comments.


================
Comment at: mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp:1012
+  // TODO: Add support for Access Qualifier.
+  assert(!operands.empty() && "No operands for processing image type");
+  if (operands.size() != 8)
----------------
ergawy wrote:
> Weiwei-2021 wrote:
> > ergawy wrote:
> > > small nit: I think the following `if` condition is more appropriate for this check. We can remove this line.
> > > 
> > > (Hope you don't mind me passing by :) ).
> > :) Thank you for reviewing!  I will remove this assertion and upload again (after I figure out how to upload a revision. This is my first time using this system - still learning. :p)
> One way to do that is:
> 
> - Do whatever changes on your working copy (i.e. remove the `assert` in this case).
> - `git add <file>` and `git commit --amend`.
> - `arc diff origin/main`, arcanist should detect that you want to update this revision (rather than open a new one).
> 
> So, basically, updating a revision is the same flow as opening a new one with the exception of amending to the already existing commit (most of the time at least). Hope that helps.
Thank you for the explanation! I updated it. 


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

https://reviews.llvm.org/D95580



More information about the llvm-commits mailing list