[PATCH] D31123: RegisterPressure: Add operators to RegisterMaskPair

Sun Mar 19 16:09:36 PDT 2017

Axel, I would subclass map instantiation and define own operator[].

Stas

--- Original message ---
From:   Axel Davy <axel.davy at ens.fr>
Sent:   March 19, 2017 4:00:49 PM
To:     Daniel Berlin <dberlin at dberlin.org>
CC:     reviews+D31123+public+a88be35608dd4bb7 at reviews.llvm.org, Matthias Braun <matze at braunis.de>, Mekhanoshin, Stanislav <Stanislav.Mekhanoshin at amd.com>, Alexander Timofeev <timofeev.alexander at gmail.com>, llvm-commits <llvm-commits at lists.llvm.org>
Subject:        Re: [PATCH] D31123: RegisterPressure: Add operators to RegisterMaskPair

Thanks.

This looks like a lot of code just to replace two lines though, should I really prefer this code over the previous one ?

Do you have suggestion to remove the errors when the the constructor RegisterMaskPair() is not added ?

Compilation gives

In file included from /home/axel/packages/llvm/llvm/include/llvm/PassRegistry.h:20:0,
                 from /home/axel/packages/llvm/llvm/include/llvm/PassSupport.h:27,
                 from /home/axel/packages/llvm/llvm/include/llvm/Pass.h:387,
                 from /home/axel/packages/llvm/llvm/include/llvm/IR/DataLayout.h:27,
                 from /home/axel/packages/llvm/llvm/include/llvm/Target/TargetMachine.h:19,
                 from /home/axel/packages/llvm/llvm/lib/Target/AMDGPU/AMDGPU.h:14,
                 from /home/axel/packages/llvm/llvm/lib/Target/AMDGPU/SIMachineScheduler.cpp:15:
/home/axel/packages/llvm/llvm/include/llvm/ADT/DenseMap.h: Dans l'instanciation de « BucketT* llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::InsertIntoBucket(BucketT*, KeyArg&&, ValueArgs&& ...) [with KeyArg = const unsigned int&; ValueArgs = {}; DerivedT = llvm::DenseMap<unsigned int, llvm::RegisterMaskPair>; KeyT = unsigned int; ValueT = llvm::RegisterMaskPair; KeyInfoT = llvm::DenseMapInfo<unsigned int>; BucketT = llvm::detail::DenseMapPair<unsigned int, llvm::RegisterMaskPair>] » :
/home/axel/packages/llvm/llvm/include/llvm/ADT/DenseMap.h:270:12:   requis par « llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::value_type& llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::FindAndConstruct(const KeyT&) [with DerivedT = llvm::DenseMap<unsigned int, llvm::RegisterMaskPair>; KeyT = unsigned int; ValueT = llvm::RegisterMaskPair; KeyInfoT = llvm::DenseMapInfo<unsigned int>; BucketT = llvm::detail::DenseMapPair<unsigned int, llvm::RegisterMaskPair>; llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::value_type = llvm::detail::DenseMapPair<unsigned int, llvm::RegisterMaskPair>] »
/home/axel/packages/llvm/llvm/include/llvm/ADT/DenseMap.h:274:28:   requis par « ValueT& llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::operator[](const KeyT&) [with DerivedT = llvm::DenseMap<unsigned int, llvm::RegisterMaskPair>; KeyT = unsigned int; ValueT = llvm::RegisterMaskPair; KeyInfoT = llvm::DenseMapInfo<unsigned int>; BucketT = llvm::detail::DenseMapPair<unsigned int, llvm::RegisterMaskPair>] »
/home/axel/packages/llvm/llvm/lib/Target/AMDGPU/SIMachineScheduler.cpp:2142:37:   requis depuis ici
/home/axel/packages/llvm/llvm/include/llvm/ADT/DenseMap.h:458:5: erreur : no matching function for call to « llvm::RegisterMaskPair::RegisterMaskPair() »
     ::new (&TheBucket->getSecond()) ValueT(std::forward<ValueArgs>(Values)...);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/axel/packages/llvm/llvm/include/llvm/CodeGen/MachineScheduler.h:87:0,
                 from /home/axel/packages/llvm/llvm/lib/Target/AMDGPU/SIMachineScheduler.h:20,
                 from /home/axel/packages/llvm/llvm/lib/Target/AMDGPU/SIMachineScheduler.cpp:17:
/home/axel/packages/llvm/llvm/include/llvm/CodeGen/RegisterPressure.h:42:3: note : candidate: llvm::RegisterMaskPair::RegisterMaskPair(unsigned int, llvm::LaneBitmask)
   RegisterMaskPair(unsigned RegUnit, LaneBitmask LaneMask)
   ^~~~~~~~~~~~~~~~
/home/axel/packages/llvm/llvm/include/llvm/CodeGen/RegisterPressure.h:42:3: note :   candidate expects 2 arguments, 0 provided
/home/axel/packages/llvm/llvm/include/llvm/CodeGen/RegisterPressure.h:38:8: note : candidate: constexpr llvm::RegisterMaskPair::RegisterMaskPair(const llvm::RegisterMaskPair&)
 struct RegisterMaskPair {
        ^~~~~~~~~~~~~~~~
/home/axel/packages/llvm/llvm/include/llvm/CodeGen/RegisterPressure.h:38:8: note :   candidate expects 1 argument, 0 provided
/home/axel/packages/llvm/llvm/include/llvm/CodeGen/RegisterPressure.h:38:8: note : candidate: constexpr llvm::RegisterMaskPair::RegisterMaskPair(llvm::RegisterMaskPair&&)
/home/axel/packages/llvm/llvm/include/llvm/CodeGen/RegisterPressure.h:38:8: note :   candidate expects 1 argument, 0 provided

On 19/03/2017 23:43, Daniel Berlin wrote:

On Sun, Mar 19, 2017 at 3:28 PM, Axel Davy <axel.davy at ens.fr<mailto:axel.davy at ens.fr>> wrote:
I'm not familiar with std::transform, but it looks like one element in the source
can only give one element in the destination, whereas here you can have one element
decomposing into several ones.

std::transform does not require the input == the output type.

It only requires that assignment of the output iterator takes the type

Thus, the following should work:

std::transform(Regs.begin(), Regs.end(), vector_inserter(Result), [&] (const RegisterMaskPair &RegPair)
{
return getPairsForReg(RegPair.RegUnit,  RegPair.LaneMask)
});

Where vector_inserter is defined as:
class vector__inserter
    : public std::iterator<std::output_iterator_tag, void, void, void, void> {
private:
  SmallVectorImpl<RegisterMaskPair> &Result;

public:
  explicit op_inserter(SmallVectorImpl<RegisterMaskPair> &Result) : Result(Result) {}

  op_inserter &operator=(const SmallVectorImpl<RegisterMaskPair> &Val) {
    Result.insert(Result.end(), Val.begin(), Val.end())
    return *this;
  }
  vector_inserter &operator*() { return *this; }
  vector_inserter &operator++() { return *this; }
  vector_inserter &operator++(int) { return *this; }
};

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170319/72cca226/attachment.html>