In D61321#1546826, @arsenm wrote:

We could try to be heroic and use something stronger than unsigned for registers

Are you talking about using stronger types for registers just in the Builder interface? What would the usage look like? My big concern (besides the massive change this would take) is how verbose it would be to use the APIs.

I mean everywhere, but that's a huge project. I imagine a struct Register { unsigned X; isVirtual(); isPhysical() }

Clarify the legality of G_BITCAST where non-trivial bitcasts appear.

In D62423#1517863, @Petar.Avramovic wrote:

Producer/Consumer type set and minimum rules for scalars are assuming that set of available integer scalars is the same as set of all available floating point scalars. This does not have to be the case.

LGTM

In D61787#1527002, @Petar.Avramovic wrote:

Ping.

Update to account for review comments so far except for the non-integral
pointers one for which I need more information.

Hi Petar,

G_MERGE_VALUES always has scalars
Fixed a else after return

Small comment fix

Sorry for the slow reply. I had to work on other things for a couple days.

LGTM

Additional test and small nits

In D61289#1493275, @rovka wrote:

Thanks for the explanations. I think you have some good points. Overall, it still looks to me like we're complicating things for backend writers and introducing a lot of subtle distinctions to keep in mind. It would be useful to hear what other people think about this.

In D61289#1493205, @Petar.Avramovic wrote:

Perfect. What about G_ZEXT_INREG since it is more efficient to narrow scalar G_ZEXT_INREG then bitwise instruction with some bit mask?

Added example and further explanation of the rule processing.
Subdivided into sections and other minor changes.

Expanded on the use of G_SEXT_INREG
Typos, nits, and other minor changes

In D61321#1484604, @arsenm wrote:

For the use cases, I think this should just be int64_t instead of APInt

In D61289#1483719, @arsenm wrote:

Is the intention to now use this instead of the current system of relying on the cleanup of legalization artifacts?

LGTM

LGTM

In D58088#1405089, @glenjofe wrote:

That looks simple enough. I haven't gone through the spec but something as simple as:

namespace llvm {
template<class Ptr>
auto to_address(const Ptr &P) noexcept { return P.operator->(); }
template<class T>
constexpr T* to_address(T* P) noexcept { return P; }
}

seems to cover the majority of it (and definitely the bits I need). It's just the std::pointer_traits<Ptr>::to_address(p) based implementation of the fancy pointer overload that's missing.

If it is named to_address and intended to convert any pointer to a raw pointer then it should be:

namespace llvm {
template<class T>
constexpr T* to_address(T* p) noexcept { return p; }
template<class Ptr>
typename std::pointer_traits<Ptr>::element_type* to_address(const Ptr &p) noexcept { return llvm::to_address(p.operator->()); }
}

In D58088#1404872, @dblaikie wrote:

In D58088#1404769, @dsanders wrote:

I just noticed a big simplification that doesn't need C++20 features or new classes. This:

template <typename WrappedIteratorT,
          typename T1 = typename std::remove_reference<decltype(**std::declval<WrappedIteratorT>())>::type,
          typename T2 = typename std::add_pointer<T1>::type>
using raw_pointer_iterator = pointer_iterator<pointee_iterator<WrappedIteratorT, T1>, T2>;

is functionally equivalent to the raw_pointer_iterator in this patch. pointee_iterator derefs the unique_ptr, while pointer_iterator produces the raw_pointer. If that version sounds good to you then I think it makes sense to revert the llvm::to_address() patch, switch to this implementation and fold this into the GISel Combiner patch as at that point it's too trivial to warrant a dedicated patch. Does that sound good?

Beyond probably some annoying compile-time complexity/overhead (& maybe similar debugging confusion with the extra layers of abstraction) - I'm guessing this wouldn't support null pointers (because pointee_iterator probably dereferences unconditionally?) which would be a bit problematic, I'd reckon?

I just noticed a big simplification that doesn't need C++20 features or new classes. This:

template <typename WrappedIteratorT,
          typename T1 = typename std::remove_reference<decltype(**std::declval<WrappedIteratorT>())>::type,
          typename T2 = typename std::add_pointer<T1>::type>
using raw_pointer_iterator = pointer_iterator<pointee_iterator<WrappedIteratorT, T1>, T2>;

is functionally equivalent to the raw_pointer_iterator in this patch. pointee_iterator derefs the unique_ptr, while pointer_iterator produces the raw_pointer. If that version sounds good to you then I think it makes sense to revert the llvm::to_address() patch, switch to this implementation and fold this into the GISel Combiner patch as at that point it's too trivial to warrant a dedicated patch. Does that sound good?

Late bound return, remove noexcept, and update comments to match

The std::to_address implementation is in https://reviews.llvm.org/D58421. I had to add another limitation to it as it accidentally used C++14 which prevents the deduction of the return type.