[PATCH] D106294: [flang] Implement the runtime portion of the UNPACK intrinsic
Pete Steinfeld via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 20 09:49:34 PDT 2021
PeteSteinfeld added inline comments.
================
Comment at: flang/runtime/transformational.cpp:524
mask.IncrementSubscripts(maskAt);
- field.IncrementSubscripts(fieldAt);
+ if (field.rank() != 0) { // scalar "field" argument
+ field.IncrementSubscripts(fieldAt);
----------------
jeanPerier wrote:
> mleair wrote:
> > PeteSteinfeld wrote:
> > > mleair wrote:
> > > > Same comment as before. llvm style does not like braces for single code blocks.
> > > And as I mentioned in the other pull request, I don't think we're following this convention for flang code.
> > Saw you comment in your cshift review. So, disregard.
> It looks ok to me to have it the way you changed it, but I am surprised you had to change it. Isn't `field.IncrementSubscripts(fieldAt)` a no-op on scalar `field` ? Did you run into bugs without your change ?
This is weird. I thought I had a test that failed without this change, but I just tried to reproduce it, and I can't. So I'll revert the code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106294/new/
https://reviews.llvm.org/D106294
More information about the llvm-commits
mailing list