Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
  • Sign in / Register
  • GHC GHC
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Locked Files
  • Issues 5,359
    • Issues 5,359
    • List
    • Boards
    • Service Desk
    • Milestones
    • Iterations
  • Merge requests 571
    • Merge requests 571
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
    • Test Cases
  • Deployments
    • Deployments
    • Releases
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Code review
    • Insights
    • Issue
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • Glasgow Haskell CompilerGlasgow Haskell Compiler
  • GHCGHC
  • Merge requests
  • !4362

Fix 64bit comparisons on 32bit x86

  • Review changes

  • Download
  • Email patches
  • Plain diff
Closed Andreas Klebinger requested to merge wip/andreask/32bit_cmp_fix into master Oct 27, 2020
  • Overview 7
  • Commits 1
  • Pipelines 5
  • Changes 6

We used to do 64bit comparisons like this:

  ChildCode64 code1 r1_lo <- iselExpr64 e1
  ChildCode64 code2 r2_lo <- iselExpr64 e2
  let r1_hi = getHiVRegFromLo r1_lo
      r2_hi = getHiVRegFromLo r2_lo
      cond = machOpToCond mop
      Just cond' = maybeFlipCond cond
  --TODO: Update CFG for x86
  let code = code1 `appOL` code2 `appOL` toOL [
        CMP II32 (OpReg r2_hi) (OpReg r1_hi),
        JXX cond true,
        JXX cond' false,
        CMP II32 (OpReg r2_lo) (OpReg r1_lo),
        JXX cond true] `appOL` genBranch false
  return code

This is subtly wrong.

If we have >= as comparison and the high bits are equal we immediately jump to true. But in fact the low bits might still go either way! We actually have to look at them to make a final decision but we (currently) don't.

This patch fixes the logic and also rewrites it to make do with a single conditional jump.

Edited Dec 13, 2020 by Ben Gamari
Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: wip/andreask/32bit_cmp_fix