weixin_39926311
weixin_39926311
2020-11-21 20:03

Implement a treetop order extended block iterator

Implement an iterator which traverses extended basic blocks in a treetop ordering, where treetop order means the order in which the extended basic blocks appear in a trace log file.

Signed-off-by: Filip Jeremic

该提问来源于开源项目:eclipse/omr

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

12条回答

  • weixin_39926311 weixin_39926311 5月前

    any other concerns?

    点赞 评论 复制链接分享
  • weixin_39600400 weixin_39600400 5月前

    LGTM

    点赞 评论 复制链接分享
  • weixin_39926311 weixin_39926311 5月前

    Unmarking WIP. This is ready to be reviewed. Consumer of this code can be found in https://github.com/eclipse/openj9/pull/537.

    点赞 评论 复制链接分享
  • weixin_39600400 weixin_39600400 5月前

    how is textual order different than treetop order/order of evaluation/order of execution? This is more correctly an extended basic block traversal in treetop order no? Having an iterator that claims to work like an unrelated piece of RAS infrastructure seems dangerous - I'd postulate we need either a connection between the RAS infra and this if the connection to RAS is important or the definition and name of the traversal should reflect the true implementation - namely in terms of treetops and/or the order of evaluation.

    点赞 评论 复制链接分享
  • weixin_39926311 weixin_39926311 5月前

    how is textual order different than treetop order/order of evaluation/order of execution? This is more correctly an extended basic block traversal in treetop order no? Having an iterator that claims to work like an unrelated piece of RAS infrastructure seems dangerous - I'd postulate we need either a connection between the RAS infra and this if the connection to RAS is important or the definition and name of the traversal should reflect the true implementation - namely in terms of treetops and/or the order of evaluation.

    I based my commit off the following comment in OMRBlock.hpp: https://github.com/eclipse/omr/blob/5003b868c5bdf849cc2f19fde9b0cc10eb880824/compiler/il/OMRBlock.hpp#L87-L98

    Is this comment incorrect? I used the word textual as it is defined in the above comment. I'm happy to change the naming to something different if this is incorrect (in which case it also needs to be addressed). Do you have any suggestions?

    点赞 评论 复制链接分享
  • weixin_39600400 weixin_39600400 5月前

    The comment in OMRBlock.hpp is correct - though the textual notion there is a point of clarification - the blocks are either walked in treetop order or in CFG order (where the two must coincide for the contents of an extended basic block). I would prefer to avoid a notion of textual escaping more generally as a concept - treetop order would seem more correct to me, but happy to hear the view of others.

    The thing I think that confused me a bit more was that I'm not sure if an

    ExtendedBlockIterator
    should be traversing the blocks in an extended block or traversing the extended blocks of the CFG...

    Naming is never my strong point - sadly. The best I can come up with is a very clumsy

    TreeTopOrderExtendedBlockEntryIterator
    which is an awful mouthful and not really an improvement, but seems more precise...
    点赞 评论 复制链接分享
  • weixin_39926311 weixin_39926311 5月前

    Naming is never my strong point - sadly. The best I can come up with is a very clumsy TreeTopOrderExtendedBlockEntryIterator which is an awful mouthful and not really an improvement, but seems more precise...

    Fixed in 355742b

    点赞 评论 复制链接分享
  • weixin_39604092 weixin_39604092 5月前

    fwiw, and perhaps I'm desensitized to awkwardly descriptive names in TR, but I don't find TreeTopOrderExtendedBlockEntryIterator too bad at all. :-)

    点赞 评论 复制链接分享
  • weixin_39604092 weixin_39604092 5月前

    genie-omr build all

    点赞 评论 复制链接分享
  • weixin_39604092 weixin_39604092 5月前

    Given that this is standalone infrastructure that is not connected nor exercised within OMR I would like to see an issue opened to track the creation of an FV test case that will exercise its behaviour. Such a test could manually construct blocks in a particular order and then leverage this iterator to ensure the blocks are visited in the TreeTop order that we expect.

    点赞 评论 复制链接分享
  • weixin_39604092 weixin_39604092 5月前

    genie-omr build all

    点赞 评论 复制链接分享
  • weixin_39604092 weixin_39604092 5月前

    Thanks for the changes. I'll wait for 's +1 before merging.

    点赞 评论 复制链接分享

相关推荐