eclipse / omr

Eclipse OMR™ Cross platform components for building reliable, high performance language runtimes
http://www.eclipse.org/omr
Other
940 stars 394 forks source link

checkAndReplaceRotation obsolete comments? #4472

Open JamesKingdon opened 4 years ago

JamesKingdon commented 4 years ago

While working on checkAndReplaceRotation I noticed some comments that appear (hopefully) to be obsolete. Can we confirm that these are no longer relevant and delete them?

There are a few lines of dead code starting at 4626:

https://github.com/eclipse/omr/blob/master/compiler/optimizer/OMRSimplifierHandlers.cpp#L4626-L4632

and a more worrisome trailing comment at 4666:

https://github.com/eclipse/omr/blob/master/compiler/optimizer/OMRSimplifierHandlers.cpp#L4666

github-actions[bot] commented 4 years ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment on the issue or it will be closed in 60 days.

JamesKingdon commented 4 years ago

Above links are out of date as the code has moved. Not sure how best to avoid this, so here's some cut & paste to search for:

   // To simplify my code, I am not going to handle these two cases on the assumption that simplifier will already have simplified these away.
   //   if (firstChild->getOpCode().isLeftShift() && !secondChild->getOpCode().isShiftLogical() && !secondChild->getOpCode().isRightShift())
   //      return false;

   //   if ( secondChild->getOpCode().isLeftShift() && !firstChild->getOpCode().isShiftLogical() && !firstChild->getOpCode().isRightShift())
   //      return false;

TR::Node::recreate(node, TR::ILOpCode::getRotateOpCodeFromDt(mulConstNode->getDataType())); // this line will have to change based on T

fjeremic commented 4 years ago

@JamesKingdon the way to avoid this is to use GitHub permalinks. If you visit either of the two links you have and click y on your keyboard the URL will change to a permalink. You can also do this by clicking the three dots ... next to the highlighted lines and copying the permalink from there.

Myself I use a nifty extension called Refined GitHub [1] (I highly recommend!) which automatically (among many many other things) examines non-permalinked lines of code with the date of when the comment was posted and alters them to permalinks. Here are the permalinks in your original post with altered URLs:

There are a few lines of dead code starting at 4626:

https://github.com/eclipse/omr/blob/HEAD@%7B2019-10-16T18:42:12Z%7D/compiler/optimizer/OMRSimplifierHandlers.cpp#L4626-L4632

and a more worrisome trailing comment at 4666:

https://github.com/eclipse/omr/blob/HEAD@%7B2019-10-16T18:42:12Z%7D/compiler/optimizer/OMRSimplifierHandlers.cpp#L4666

[1] https://github.com/sindresorhus/refined-github

JamesKingdon commented 4 years ago

Thanks Filip, I thought I always used permalinks, but I must have slipped up when I raised this issue. Update: Thanks for the link to Refined GitHub, that looks really useful!