search

captainys / TOWNSEMU

FM Towns Emulator "Tsugaru"
BSD 3-Clause "New" or "Revised" License
238 stars 17 forks source link

Fix ror, rol #36

Closed pinterior closed 2 years ago

pinterior commented 2 years ago

以下の問題の修正です:

captainys commented 2 years ago

発見していただいてどうもありがとうございます！ROL, RORでCFが立たなかった原因は、元のコードではマスク後のカウンタがゼロなら立てないことにしていましたが、実際はマスク前のカウンタをチェックするべきだった、という問題でしたか?

pinterior commented 2 years ago

そうです。>マスク前のカウンタをチェックするべきだった Intel® 64 and IA-32 Architectures Software Developer’s Manuals でも MOD SIZE した結果の tempCOUNT ではなく、下位5ビットの COUNT & COUNTMASK を参照しています。

書き忘れていましたが、ror の修正で RUMSTORM が起動時にPage Faultしなくなります。 (LZW伸長前に ror などを使用してデスクランブルしていたため、期待と異なるデータを入力にして異常動作していたようです)

captainys commented 2 years ago

おおなるほど！了解しました。一点だけ細かいことですが、 auto e = c % std::numeric_limits::digits; これは多分オプティマイザが自動的に c&(std::numeric_limits::digits-1) に置き換えてしまうとは思うのですが、できる限り割り算を避けるように書いているので、明示的に、 auto e = c & (std::numeric_limits::digits-1); と書いてはどうかと思うのですが、いかがでしょうか?

pinterior commented 2 years ago

除数が2のべきでない定数の場合に除算を乗算とシフトに変換するのは絶対にやらないわけですから、 2のべきの場合も含めて全部コンパイラに任せてしまったほうがいいというのが個人的な考えですね。 (被除数の符号の有無には注意する必要がありますが)

とはいえもう1点気になっていた点もあったので、変更しておきます。

captainys commented 2 years ago

あ、エラーのメールが届いてたらごめんなさい。さっきPUSHしたときLinuxのBuildが壊れたままでした。それはこっちで直しますんで。

captainys commented 2 years ago

&の件了解しました。理想的には、8, 16, 32以外の値だった場合はコンパイル時にチェックしてエラーが出るようにできれば、&にしてしまっても安全と思いますが、現実的にはオプティマイザが自動的にやってしまうでしょうし、ここはpinさんのコードなのでpinさんの流儀に従いましょう。Mergeさせていただきます。ありがとうございます！