[PATCH] D152655: [MC] Merge MC[Sub,Super]RegIterator with mc_[sub,super]_reg_iterator
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 15 14:19:49 PDT 2023
dblaikie accepted this revision.
dblaikie added inline comments.
This revision is now accepted and ready to land.
================
Comment at: llvm/include/llvm/MC/MCRegisterInfo.h:511
+ // Cache the current value, so that we can return a reference to it.
+ MCPhysReg Val;
+
----------------
barannikov88 wrote:
> barannikov88 wrote:
> > @dblaikie Could you comment on this?
> > This requires type type to be default constructible. In D152098 I add MCRegUnit that does not have a good default value. Maybe I should instead return a proxy object from `operator*` that will hold the value? Would that be correct w.r.t. iterator requirements? Probably factor the proxy out into a common (templated) base class, too.
> >
> Ah I guess this won't change a thing, I would still need to be able to construct the proxy from some value, which I don't have in case of the end iterator.
>
If you need something that isn't default constructible/can't just have an instance hanging around as a member like this, the next best thing would be `std::optional<T>` so it can be non-constructed. If the overhead of the extra bool/padding in `std::optional` is significant, you could maybe use a raw aligned buffer and /maybe/ conditionalize on the validity of the underlying iterator to decide whether there's an object in that buffer or not. But ensuring the code doesn't leak/fail to construct/destroy properly would be subtle.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152655/new/
https://reviews.llvm.org/D152655
More information about the llvm-commits
mailing list