nod-ai / iree-amd-aie

IREE plugin repository for the AMD AIE accelerator
Apache License 2.0
69 stars 30 forks source link

Bump IREE 10/09/2024 #837

Closed makslevental closed 1 month ago

makslevental commented 1 month ago

This is ahead of schedule I guess but I'd like to get this PR locally https://github.com/iree-org/iree/pull/18675 (the PR built locally with no changes).

newling commented 1 month ago
  690 |     options.clAggressiveMode = ArrayRef(mode);
      |     ~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~

My interpretation of this code is that it has UB, and it'll be a heavy lift to remove the UB, but if you don't mind leaving the UB then it shouldn't be too hard.

makslevental commented 1 month ago

My interpretation of this code is that it has UB, and it'll be a heavy lift to remove the UB, but if you don't mind leaving the UB then it shouldn't be too hard.

Why? there's an arrayref ctor from std::vec. Not sure why it wasn't being found but let's see if assignment ctor works.

Oh you're saying because the std::vec gets dropped almost immediately? That's not UB just use-after-free? yea that's probably true - soln is just to do static std::vector<std::string> mode;.

newling commented 1 month ago

My interpretation of this code is that it has UB, and it'll be a heavy lift to remove the UB, but if you don't mind leaving the UB then it shouldn't be too hard.

Why? there's an arrayref ctor from std::vec. Not sure why it wasn't being found but let's see if assignment ctor works.

Oh you're saying because the std::vec gets dropped almost immediately? That's not UB just use-after-free? yea that's probably true - soln is just to do static std::vector<std::string> mode;.

Maybe. Something like

 static const std::vector<std::string> modeA {"L1"}; 
 static const std::vector<std::string> modeB {};
 const std::vector<std::string> & mode = x ? modeA : modeB;

to make it less scary when it's not just a single threads, single call, single etc.

makslevental commented 1 month ago

static const std::vector modeA {"L1"}; static const std::vector modeB {}; const std::vector & mode = x ? modeA : modeB;



to make it less scary when it's not just a single threads, single call, single etc.

you don't need the empty vec - that's just "CLI arg not passed" right? anyway assignment didn't work from vec but there's this std::initializer_list ctor that should work

newling commented 1 month ago

static const std::vectorstd::string modeA {"L1"}; static const std::vectorstd::string modeB {}; const std::vectorstd::string & mode = x ? modeA : modeB;


to make it less scary when it's not just a single threads, single call, single etc.

you don't need the empty vec - that's just "CLI arg not passed" right? anyway assignment didn't work from vec but there's this std::initializer_list ctor that should work

Makes sense, so no need for this indirection, good point